[PATCH 3/4] mtd: Fix a potential NULL ptr deref bug and mem leak in init_msp_flash()

Jesper Juhl jesper.juhl at gmail.com
Sat Aug 25 21:56:17 EDT 2007


    mtd: Fix a potential NULL ptr deref bug and mem leak in init_msp_flash()
    
    In drivers/mtd/maps/pmcmsp-flash.c::init_msp_flash() there is
    currently this funky little piece of code:
    
           msp_maps[i].name = strncpy(kmalloc(7, GFP_KERNEL),
                                   flash_name, 7);
    
    If this (tiny) memory allocation should happen to fail, then
    strncpy() will result in bad things happening - much better to
    simply check the allocation and return a sensible error than crash
    the entire kernel on a NULL deref.
    
    While fixing the above I also reorganized some other lines of code
    in the neighbourhood. There is a nice little check of whether or
    not the ioremap() returns NULL, but the check happens after we have
    already done the kmalloc() described above, so in case it triggers
    it will cause us to leak the 7 bytes that kmalloc() allocated. This
    is easily avoidable by simply moving the check so that if ioremap()
    fails we don't even attempt to do the memory allocation.
    And while I was moving code around I also moved the setting of
    msp_maps[i].bankwidth to 1 below both the ioremap() and kmalloc() so
    that if we bail out due to either of them failing then we don't have
    to spend time doing that assignment - very unlikely to actually
    matter in real life, but it seemed such an obvious
    micro-optimization that I just figured I might as well do it.
    
    Signed-off-by: Jesper Juhl <jesper.juhl at gmail.com>
---
 drivers/mtd/maps/pmcmsp-flash.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/maps/pmcmsp-flash.c b/drivers/mtd/maps/pmcmsp-flash.c
index af72cdd..b6d382a 100644
--- a/drivers/mtd/maps/pmcmsp-flash.c
+++ b/drivers/mtd/maps/pmcmsp-flash.c
@@ -115,14 +115,18 @@ int __init init_msp_flash(void)
 		 */
 		if (size > CONFIG_MSP_FLASH_MAP_LIMIT)
 			size = CONFIG_MSP_FLASH_MAP_LIMIT;
-		msp_maps[i].virt = ioremap(addr, size);
-		msp_maps[i].bankwidth = 1;
-		msp_maps[i].name = strncpy(kmalloc(7, GFP_KERNEL),
-					flash_name, 7);
 
+		msp_maps[i].virt = ioremap(addr, size);
 		if (msp_maps[i].virt == NULL)
 			return -ENXIO;
 
+		msp_maps[i].name = kmalloc(7, GFP_KERNEL);
+		if (msp_maps[i].name == NULL)
+			return -ENOMEM;
+		strncpy(msp_maps[i].name, flash_name, 7);
+
+		msp_maps[i].bankwidth = 1;
+
 		for (j = 0; j < pcnt; j++) {
 			part_name[5] = '0' + i;
 			part_name[7] = '0' + j;





More information about the linux-mtd mailing list