diff mbox

[2/2] drivers/mtd/maps/pcmciamtd.c: coding style cleanups

Message ID 20100415172750.GB27756@comet.dominikbrodowski.net
State New, archived
Headers show

Commit Message

Dominik Brodowski April 15, 2010, 5:27 p.m. UTC 
Alexander,

On Mon, Apr 12, 2010 at 07:51:12PM +0400, Alexander Kurz wrote:
> @@ -691,7 +701,8 @@ static void pcmciamtd_detach(struct pcmcia_device *link)
>  	if(dev->mtd_info) {
>  		del_mtd_device(dev->mtd_info);
>  		map_destroy(dev->mtd_info);
> -		info("mtd%d: Removed", dev->mtd_info->index);
> +		dev_info(&dev->p_dev->dev, "mtd%d: Removed\n",
> +			 dev->mtd_info->index);
>  	}

Could you switch the ordering between map_destroy() and dev_info(), please,
to avoid an use-after-free? See the attached message by Julia Lawall, who
noted this issue first.

Best,
	Dominik
From: Julia Lawall <julia@diku.dk>

Moved the debugging message before the call to map_destroy, which frees its
argument.  The message is also slightly changed to reflect its new
position.

A simplified version of the semantic patch that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
expression E,E2;
@@

del_mtd_device(E)
...
(
  E = E2
|
* E
)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/mtd/maps/pcmciamtd.c        |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Comments

Dominik Brodowski April 15, 2010, 5:59 p.m. UTC | #1 
On Thu, Apr 15, 2010 at 07:27:50PM +0200, Dominik Brodowski wrote:
> On Mon, Apr 12, 2010 at 07:51:12PM +0400, Alexander Kurz wrote:
> > @@ -691,7 +701,8 @@ static void pcmciamtd_detach(struct pcmcia_device *link)
> >  	if(dev->mtd_info) {
> >  		del_mtd_device(dev->mtd_info);
> >  		map_destroy(dev->mtd_info);
> > -		info("mtd%d: Removed", dev->mtd_info->index);
> > +		dev_info(&dev->p_dev->dev, "mtd%d: Removed\n",
> > +			 dev->mtd_info->index);
> >  	}
> 
> Could you switch the ordering between map_destroy() and dev_info(), please,
> to avoid an use-after-free? See the attached message by Julia Lawall, who
> noted this issue first.

Actually, to avoid any of the patches getting lost, I created a branch
"pcmciamtd" at:

http://git.kernel.org/?p=linux/kernel/git/brodo/pcmcia-2.6.git;a=shortlog;h=refs/heads/pcmciamtd
diff mbox

Patch

diff --git a/drivers/mtd/maps/pcmciamtd.c b/drivers/mtd/maps/pcmciamtd.c
index 87b2b8f..3e339de 100644
--- a/drivers/mtd/maps/pcmciamtd.c
+++ b/drivers/mtd/maps/pcmciamtd.c
@@ -689,8 +689,8 @@  static void pcmciamtd_detach(struct pcmcia_device *link)
 
 	if(dev->mtd_info) {
 		del_mtd_device(dev->mtd_info);
+		info("mtd%d: Removing", dev->mtd_info->index);
 		map_destroy(dev->mtd_info);
-		info("mtd%d: Removed", dev->mtd_info->index);
 	}
 
 	pcmciamtd_release(link);