if cdev_add() fails, do you need to "put" the cdev's kobject?
still perusing various drivers to use as examples in class, and i ran across this snippet in the load routine of drivers/char/raw.c: ===== start excerpt ===== cdev_init(&raw_cdev, &raw_fops); ret = cdev_add(&raw_cdev, dev, MAX_RAW_MINORS); if (ret) { kobject_put(&raw_cdev.kobj); goto error_region; } ===== end ===== of course it's wise to check the return code from cdev_add() to make sure it worked, but is it really necessary to call kobject_put() there? i don't recall other character drivers calling that if they note that cdev_add() failed. it's probably harmless, but it can't be necessary, can it? unless that driver is doing something unusual somewhere else in its code. rday -- ======================================================================== Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday ========================================================================
On Tue, May 03, 2011 at 05:46:05PM -0400, Robert P. J. Day wrote:
still perusing various drivers to use as examples in class, and i ran across this snippet in the load routine of drivers/char/raw.c:
===== start excerpt =====
cdev_init(&raw_cdev, &raw_fops); ret = cdev_add(&raw_cdev, dev, MAX_RAW_MINORS); if (ret) { kobject_put(&raw_cdev.kobj); goto error_region; }
===== end =====
of course it's wise to check the return code from cdev_add() to make sure it worked, but is it really necessary to call kobject_put() there?
No, that is wrong. No one should ever mess with the kobject in a cdev. I want to call it something other than a kobject, as it's not really used as you would think of a kobject (it's used in the hash lookup of the object when userspace opens a chardev, it has nothing to do with sysfs.)
i don't recall other character drivers calling that if they note that cdev_add() failed. it's probably harmless, but it can't be necessary, can it? unless that driver is doing something unusual somewhere else in its code.
Hopefully not. Nice auditing, you are going to start sending patches for this, right? :) thanks, greg k-h
On Tue, 3 May 2011, Greg KH wrote:
On Tue, May 03, 2011 at 05:46:05PM -0400, Robert P. J. Day wrote:
still perusing various drivers to use as examples in class, and i ran across this snippet in the load routine of drivers/char/raw.c:
===== start excerpt =====
cdev_init(&raw_cdev, &raw_fops); ret = cdev_add(&raw_cdev, dev, MAX_RAW_MINORS); if (ret) { kobject_put(&raw_cdev.kobj); goto error_region; }
===== end =====
of course it's wise to check the return code from cdev_add() to make sure it worked, but is it really necessary to call kobject_put() there?
No, that is wrong. No one should ever mess with the kobject in a cdev. I want to call it something other than a kobject, as it's not really used as you would think of a kobject (it's used in the hash lookup of the object when userspace opens a chardev, it has nothing to do with sysfs.)
so based on the code surrounding that snippet, a quick fix for now would be to simply remove that call to kobject_put(), correct? there might be other cleanup but, for now, that removal would seem to be obvious. if so, i can submit a patch. rday -- ======================================================================== Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday ========================================================================
participants (2)
-
Greg KH -
Robert P. J. Day