[settings] Avoid returning uninitialised data on error in fetch_xxx_setting()

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.

diff --git a/src/core/settings.c b/src/core/settings.c
index 3e9eb18..f97842f 100644 (file)
--- a/src/core/settings.c
+++ b/src/core/settings.c
@@ -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;
@@ -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 ) );
 }
@@ -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] );
@@ -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;
@@ -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;
@@ -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;