From 612f4e7a990272f67dc1461e6cf1c5ad3b83dd14 Mon Sep 17 00:00:00 2001
From: Michael Brown <mcb30@etherboot.org>
Date: Wed, 22 Oct 2008 21:18:15 +0100
Subject: [PATCH] [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.
---
 src/core/settings.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/core/settings.c b/src/core/settings.c
index 3e9eb18a..f97842f4 100644
--- 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;