linux-kernel - Re: [GIT PULL] block fixes for 2.6.32-rc

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] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 28 Oct 2009 20:10:04 +0100
From:	Jens Axboe <jens.axboe@...cle.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Linux Kernel <linux-kernel@...r.kernel.org>
Subject: Re: [GIT PULL] block fixes for 2.6.32-rc

On Wed, Oct 28 2009, Linus Torvalds wrote:
> 
> 
> On Wed, 28 Oct 2009, Jens Axboe wrote:
> > 
> > Neil Brown (1):
> >       block: use after free bug in __blkdev_get
> >
> > diff --git a/fs/block_dev.c b/fs/block_dev.c
> > index 9cf4b92..8bed055 100644
> > --- a/fs/block_dev.c
> > +++ b/fs/block_dev.c
> > @@ -1248,8 +1248,8 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
> >  			bd_set_size(bdev, (loff_t)bdev->bd_part->nr_sects << 9);
> >  		}
> >  	} else {
> > -		put_disk(disk);
> >  		module_put(disk->fops->owner);
> > +		put_disk(disk);
> >  		disk = NULL;
> >  		if (bdev->bd_contains == bdev) {
> >  			if (bdev->bd_disk->fops->open) {
> 
> Is this really right? You do the module-put while the disk is still 
> available..
> 
> I get the feeling that it might have been better to do
> 
> 	struct module *mod = disk->fops->owner;
> 	put_disk(disk);
> 	module_put(mod);
> 
> instead, which tries to make sure that the module is put only after we've 
> gotten rid of the disk entirely.
> 
> But I dunno. Maybe there is some reason why it's safe either way. You're 
> sure the kobject_put() in put_disk will never call to the module?

Hmm good point. The general use case in block_dev.c is indeed to put the
module after the disk, which does seem a bit backwards (at least
logically). I'd say pull the patch since it fixes Neil's problem and
follows the general pattern, then I'll investigate whether that use
pattern is indeed safe. It wont make things worse and the current usage
being fixed is definitely wrong.

-- 
Jens Axboe

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