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]

Date:	Thu, 18 Oct 2007 16:08:13 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Kay Sievers <kay.sievers@...y.org>
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, 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.

Alan Stern

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