Re: [RFC] Changes to the driver model class code.

[Dominik Brodowski <linux@dominikbrodowski.net>]

On Tue, Mar 15, 2005 at 02:14:31PM -0800, Greg KH wrote:
> It will not make the reference counting logic easier to get wrong, or
> easier to get right.  It totally takes it away from the user, and makes
> them implement it themselves if they so wish (like the USB HCD patch
> does.)

Hi,

While looking more closely at your patches, I noticed the following race:

A) attribute is opened -> class_device's reference count is increased

B) usb/host/ohci-dbg.c::remove_debug_files() -- succeeds, as it doesn't check
   class_device's reference count()
B) usb/core/hcd.c::usb_deregister_count() -- class_device_unregister doesn't
   wait until class_device's reference count reaches zero, so 
   struct class_device still has "struct usb_bus *bus" saved as class_data
   and continues to exist.

B) possibly the kref count of struct usb_bus reaches zero, and struct usb_bus *
   is kfreed.

A) attribute is read -> e.g. usb/host/ohci-dbg.c::show_periodic()
        bus = class_get_devdata(class_dev);
        hcd = bus->hcpriv;
  --> accessing kfree'd structure. Ooops.

A) ... [if it hadn't oopsed] attribute is closed, reference count reaches zero,
   class_device is removed.


If both reference counts were kept unified (as with previous struct 
class{,_device} design) this couldn't happen. The proper reference counting
for dynamically allocated objects and their "attributes" is _the_ advantage 
of sysfs/driver model in favour of procfs.

Or am I missing something?

Thanks and Happy Easter,
	Dominik
-
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/