Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[settings] Avoid returning uninitialised data on error in fetch_xxx_s…
…etting()

Callers (e.g. usr/autoboot.c) may not check the return values from
fetch_xxx_setting(), assuming that in error cases the returned setting
value will be "empty" (for some sensible value of "empty").

In particular, if the DHCP server did not specify a next-server
address, this would result in gPXE using uninitialised data for the
TFTP server IP address.
  • Loading branch information
Michael Brown committed Oct 22, 2008
1 parent cbf9003 commit 612f4e7
Showing 1 changed file with 15 additions and 5 deletions.
20 changes: 15 additions & 5 deletions src/core/settings.c
Expand Up @@ -333,6 +333,9 @@ int fetch_setting ( struct settings *settings, struct setting *setting,
struct settings *child;
int ret;

/* Avoid returning uninitialised data on error */
memset ( data, 0, len );

/* NULL settings implies starting at the global settings root */
if ( ! settings )
settings = &settings_root;
Expand Down Expand Up @@ -381,7 +384,6 @@ int fetch_setting_len ( struct settings *settings, struct setting *setting ) {
*/
int fetch_string_setting ( struct settings *settings, struct setting *setting,
char *data, size_t len ) {
memset ( data, 0, len );
return fetch_setting ( settings, setting, data,
( ( len > 0 ) ? ( len - 1 ) : 0 ) );
}
Expand Down Expand Up @@ -417,20 +419,23 @@ int fetch_ipv4_setting ( struct settings *settings, struct setting *setting,
int fetch_int_setting ( struct settings *settings, struct setting *setting,
long *value ) {
union {
long value;
uint8_t u8[ sizeof ( long ) ];
int8_t s8[ sizeof ( long ) ];
} buf;
int len;
int i;

buf.value = 0;
/* Avoid returning uninitialised data on error */
*value = 0;

/* Fetch raw (network-ordered, variable-length) setting */
len = fetch_setting ( settings, setting, &buf, sizeof ( buf ) );
if ( len < 0 )
return len;
if ( len > ( int ) sizeof ( buf ) )
return -ERANGE;

/* Convert to host-ordered signed long */
*value = ( ( buf.s8[0] >= 0 ) ? 0 : -1L );
for ( i = 0 ; i < len ; i++ ) {
*value = ( ( *value << 8 ) | buf.u8[i] );
Expand All @@ -452,10 +457,15 @@ int fetch_uint_setting ( struct settings *settings, struct setting *setting,
long svalue;
int len;

/* Avoid returning uninitialised data on error */
*value = 0;

/* Fetch as a signed long */
len = fetch_int_setting ( settings, setting, &svalue );
if ( len < 0 )
return len;

/* Mask off sign-extended bits */
*value = ( svalue & ( -1UL >> ( sizeof ( long ) - len ) ) );

return len;
Expand All @@ -469,7 +479,7 @@ int fetch_uint_setting ( struct settings *settings, struct setting *setting,
* @ret value Setting value, or zero
*/
long fetch_intz_setting ( struct settings *settings, struct setting *setting ){
long value = 0;
long value;

fetch_int_setting ( settings, setting, &value );
return value;
Expand All @@ -484,7 +494,7 @@ long fetch_intz_setting ( struct settings *settings, struct setting *setting ){
*/
unsigned long fetch_uintz_setting ( struct settings *settings,
struct setting *setting ) {
unsigned long value = 0;
unsigned long value;

fetch_uint_setting ( settings, setting, &value );
return value;
Expand Down

0 comments on commit 612f4e7

Please sign in to comment.