linux-kernel - Re: BUG in: Driver core: convert block from raw kobjects to core devices

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]

Message-Id: <1192757236.3308.3.camel@lov.site>
Date:	Fri, 19 Oct 2007 03:27:16 +0200
From:	Kay Sievers <kay.sievers@...y.org>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	Greg KH <greg@...ah.com>,
	Kernel development list <linux-kernel@...r.kernel.org>
Subject: Re: BUG in: Driver core: convert block from raw kobjects to core
	devices


On Thu, 2007-10-18 at 16:08 -0400, Alan Stern wrote:
> On Thu, 18 Oct 2007, Kay Sievers wrote:
> 
> > On Thu, 2007-10-18 at 15:23 -0400, Alan Stern wrote:
> > > This patch (as1004) fixes a refcounting bug in the development version
> > > of the block-device core.
> > > 
> > > Signed-off-by: Alan Stern <stern@...land.harvard.edu>
> > > 
> > > ---
> > > 
> > > Kay, you have got to start testing your patches better!
> > 
> > That leaves references around for SCSI target devices. There must be a
> > bug somewhere else, if the patch isn't correct.
> > 
> > > Finding and
> > > fixing refcount errors is _not_ one of my favorite ways to pass the
> > > time.  For example, you could see what happens when you insert and
> > > unplug a USB flash disk a few times.
> > 
> > What do you see with the original version?
> 
> Note that a USB drive is treated as a SCSI device.
> 
> With the original code, I see the following sequence of events when 
> add_disk() is first called.  Values in parentheses are 
> atomic_read(disk->dev.kobj.kref.refcount) after each stage runs:
> 
> 	Entry to add_disk		(1)
> 	Call to register_disk
> 	device_add			(3)
> 	CONFIG_SYSFS_DEPRECATED is not set
> 	Call disk_sysfs_add_subdirs
> 	add disk->holder_dir		(4)
> 	add disk->slave_dir		(5)
> 	Return to register_disk
> 	get_capacity			(5)
> 	bdget_disk			(5)
> 	blkdev_get (partitions)		(8)
> 	blkdev_put			(7)
> 	Return to add_disk
> 	blk_register_queue		(9)
> 
> You can see how many references each stage takes.  Now here's the
> equivalent list for del_gendisk():
> 
> 	Entry to del_gendisk		(9)
> 	invalidate_ and delete_partition loop	(7)
> 	invalidate_partition 0		(7)
> 	Call unlink_gendisk
> 	blk_unregister_queue		(5)
> 	Return to del_gendisk
> 	unregister disk->holder_dir	(4)
> 	unregister disk->slave_dir	(3)
> 	CONFIG_SYSFS_DEPRECATED is not set
> 	device_del			(1)
> 	put_device			(0) -- oops!
> 
> Matching things up we have:
> 
> 	device_add/device_del		2 refs
> 	reg/unreg subdirs		2 refs
> 	subpartitions			2 refs
> 	reg/unreg block queue		2 refs
> 
> This accounts for everything in del_gendisk except the final
> put_device.  Evidently it doesn't belong there.  There's no matching 
> get_device in add_disk or register_disk.

Hmm, do you have kobject debugging enabled? Do you ever see something
like: "kobject sdb: cleaning up" when you remove the put_device()?

Kay

-
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/