linux-kernel - Re: [2.6.31] Memory leak in SCSI initialization?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <1254569032.7873.7.camel@concordia>
Date:	Sat, 03 Oct 2009 21:23:52 +1000
From:	Michael Ellerman <michael@...erman.id.au>
To:	James Bottomley <James.Bottomley@...e.de>
Cc:	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
	linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
	Kay Sievers <kay.sievers@...y.org>
Subject: Re: [2.6.31] Memory leak in SCSI initialization?

On Fri, 2009-10-02 at 13:30 -0500, James Bottomley wrote:
> On Tue, 2009-09-22 at 23:13 +1000, Michael Ellerman wrote:
> > On Tue, 2009-09-22 at 13:18 +0900, Tetsuo Handa wrote:
> > > Hello.
> > > 
> > > I can see below message appears for 15 times in
> > > /sys/kernel/debug/kmemleak after processing /init inside initramfs.
> > > 
> > > unreferenced object 0xdeadb5c8 (size 32):
> > >   comm "insmod", pid 543, jiffies 4294674766
> > >   backtrace:
> > >     [<c048a22c>] create_object+0x135/0x202
> > >     [<c048a31e>] kmemleak_alloc+0x25/0x49
> > >     [<c04865d9>] kmemleak_alloc_recursive+0x1c/0x22
> > >     [<c0486d33>] __kmalloc+0x6c/0xb9
> > >     [<c04f5675>] kvasprintf+0x2d/0x4a
> > >     [<c04ef5af>] kobject_set_name_vargs+0x21/0x50
> > >     [<c054bbd7>] dev_set_name+0x1a/0x1c
> > >     [<e08dc1b7>] scsi_sysfs_device_initialize+0x8b/0xe4 [scsi_mod]
> > >     [<e08d9bbf>] scsi_alloc_sdev+0x134/0x18f [scsi_mod]
> > >     [<e08d9e7a>] scsi_probe_and_add_lun+0x107/0xa98 [scsi_mod]
> > >     [<e08da946>] __scsi_scan_target+0x70/0x4b1 [scsi_mod]
> > >     [<e08dadbe>] scsi_scan_channel+0x37/0x60 [scsi_mod]
> > >     [<e08dae9f>] scsi_scan_host_selected+0xb8/0xf1 [scsi_mod]
> > >     [<e08daf2c>] do_scsi_scan_host+0x54/0x5d [scsi_mod]
> > >     [<e08db2ef>] scsi_scan_host+0x14d/0x165 [scsi_mod]
> > >     [<e0959771>] mptspi_probe+0x2cd/0x2f8 [mptspi]
> > 
> > I think this will fix it:
> > 
> > diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> > index 9ce5c34..284bcbe 100644
> > --- a/drivers/scsi/scsi_scan.c
> > +++ b/drivers/scsi/scsi_scan.c
> > @@ -957,7 +957,7 @@ static inline void scsi_destroy_sdev(struct scsi_device *sdev)
> >         if (sdev->host->hostt->slave_destroy)
> >                 sdev->host->hostt->slave_destroy(sdev);
> >         transport_destroy_device(&sdev->sdev_gendev);
> > -       put_device(&sdev->sdev_gendev);
> > +       put_device(&sdev->sdev_dev);
> >  }
> >  
> >  #ifdef CONFIG_SCSI_LOGGING
> > 
> > 
> > sdev_dev has class == sdev_class. The release function for sdev_class is
> > scsi_device_cls_release(), and _that_ does a put on the sdev_gendev.
> > 
> > But someone who groks all that mess, er beauty ;D, should check that
> > makes sense.
> 
> The fix is correct, but it's depending on nastily deep magic inside the
> scsi sysfs layers.

Indeed, thanks for having a better look at it.

I also used CONFIG_DEBUG_KOBJECT to verify that there was only one name
being freed without the patch and two with it - which is a slightly more
direct proof than the memleak output.

cheers

Download attachment "signature.asc" of type "application/pgp-signature" (198 bytes)