[Skiboot] [PATCH] external/common: Fix callers of file_init_path()

Cyril Bur cyril.bur at au1.ibm.com
Mon Mar 21 11:17:52 AEDT 2016


The arch_flash_init() in arch_flash_x86.c doesn't actually check the return value
of file_init_path(), rather it is comparing the returned structure against NULL.
It is unsafe (and incorrect at the moment) to assume that file_init_path will
NULL this value on failure, it doesn't have to as it returns a value to
indicate success or failure.

The arch_flash_init() in arch_flash_powerpc.c calls file_init_path() through
another function which will return a pointer (or NULL on failure), this
function doesn't explicitly NULL its return pointer in the case that
file_init_path() fails. It has initialised the pointer to NULL so the case may
be less severe (compared to the arch_flash_x86 problem) as file_init_path()
shouldn't have changed it on failure case, however, assuming that it won't
is unsafe. It is best to explicitly NULL the return pointer if file_init_path()
returns a failure.

Signed-off-by: Cyril Bur <cyril.bur at au1.ibm.com>
---
 external/common/arch_flash_powerpc.c | 4 +++-
 external/common/arch_flash_x86.c     | 5 +++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/external/common/arch_flash_powerpc.c b/external/common/arch_flash_powerpc.c
index 19dfec8..7ce962e 100644
--- a/external/common/arch_flash_powerpc.c
+++ b/external/common/arch_flash_powerpc.c
@@ -200,7 +200,9 @@ static struct blocklevel_device *arch_init_blocklevel(const char *file, bool kee
 			return NULL;
 	}
 
-	file_init_path(file ? file : real_file, NULL, keep_alive, &new_bl);
+	rc = file_init_path(file ? file : real_file, NULL, keep_alive, &new_bl);
+	if (rc)
+		new_bl = NULL;
 	free(real_file);
 	return new_bl;
 }
diff --git a/external/common/arch_flash_x86.c b/external/common/arch_flash_x86.c
index 3be05df..0146243 100644
--- a/external/common/arch_flash_x86.c
+++ b/external/common/arch_flash_x86.c
@@ -30,6 +30,7 @@
 
 int arch_flash_init(struct blocklevel_device **r_bl, const char *file, bool keep_alive)
 {
+	int rc;
 	struct blocklevel_device *new_bl;
 
 	/* Must have passed through a file to operate on */
@@ -38,8 +39,8 @@ int arch_flash_init(struct blocklevel_device **r_bl, const char *file, bool keep
 		return -1;
 	}
 
-	file_init_path(file, NULL, keep_alive, &new_bl);
-	if (!new_bl)
+	rc = file_init_path(file, NULL, keep_alive, &new_bl);
+	if (rc)
 		return -1;
 
 	*r_bl = new_bl;
-- 
2.7.3



More information about the Skiboot mailing list