public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
From: Jesper Juhl <jesper.juhl@gmail.com>
To: David Woodhouse <dwmw2@infradead.org>
Cc: Denys Vlasenko <vda.linux@googlemail.com>,
	Jesper Juhl <jesper.juhl@gmail.com>,
	Marc St-Jean <stjeanma@pmc-sierra.com>,
	linux-mtd@lists.infradead.org,
	"Robert P. J. Day" <rpjday@mindspring.com>
Subject: [PATCH 3/4] mtd: Fix a potential NULL ptr deref bug and mem leak in init_msp_flash()
Date: Sun, 26 Aug 2007 03:56:17 +0200	[thread overview]
Message-ID: <200708260356.17664.jesper.juhl@gmail.com> (raw)
In-Reply-To: <200708260355.20916.jesper.juhl@gmail.com>


    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@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;

  reply	other threads:[~2007-08-26  2:06 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-08-26  1:52 [PATCH 0/4] mtd: cleanups and small bug fixes for init_msp_flash() Jesper Juhl
2007-08-26  1:54 ` [PATCH 1/4] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c Jesper Juhl
2007-08-26  1:55   ` [PATCH 2/4] mtd: convert some kmalloc()+memset() calls to kcalloc() " Jesper Juhl
2007-08-26  1:56     ` Jesper Juhl [this message]
2007-08-26  1:57       ` [PATCH 4/4] mtd: Check for allocation failures and bail out appropriately in init_msp_flash() Jesper Juhl
2007-08-26  8:36   ` [PATCH 1/4] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c Robert P. J. Day
2007-08-26 13:19     ` Jesper Juhl
2007-08-26 15:41       ` Robert P. J. Day

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200708260356.17664.jesper.juhl@gmail.com \
    --to=jesper.juhl@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=rpjday@mindspring.com \
    --cc=stjeanma@pmc-sierra.com \
    --cc=vda.linux@googlemail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox