On 25/02/2017 at 23:38:02 -0700, Logan Gunthorpe wrote: > Credit for this patch goes is shared with Dan Williams [1]. I've > taken things one step further to make the helper function more > useful and clean up calling code. > > There's a common pattern in the kernel whereby a struct cdev is placed > in a structure along side a struct device which manages the life-cycle > of both. In the naive approach, the reference counting is broken and > the struct device can free everything before the chardev code > is entirely released. > > Many developers have solved this problem by linking the internal kobjs > in this fashion: > > cdev.kobj.parent = &parent_dev.kobj; > > The cdev code explicitly gets and puts a reference to it's kobj parent. > So this seems like it was intended to be used this way. Dmitrty Torokhov > first put this in place in 2012 with this commit: > > 2f0157f char_dev: pin parent kobject > > and the first instance of the fix was then done in the input subsystem > in the following commit: > > 4a215aa Input: fix use-after-free introduced with dynamic minor changes > > Subsequently over the years, however, this issue seems to have tripped > up multiple developers independently. For example, see these commits: > > 0d5b7da iio: Prevent race between IIO chardev opening and IIO device > (by Lars-Peter Clausen in 2013) > > ba0ef85 tpm: Fix initialization of the cdev > (by Jason Gunthorpe in 2015) > > 5b28dde [media] media: fix use-after-free in cdev_put() when app exits > after driver unbind > (by Shauh Khan in 2016) > > This technique is similarly done in at least 15 places within the kernel > and probably should have been done so in another, at least, 5 places. > The kobj line also looks very suspect in that one would not expect > drivers to have to mess with kobject internals in this way. > Even highly experienced kernel developers can be surprised by this > code, as seen in [2]. > > To help alleviate this situation, and hopefully prevent future > wasted effort on this problem, this patch introduces a helper function > to register a char device along with its parent struct device. > This creates a more regular API for tying a char device to its parent > without the developer having to set members in the underlying kobject. > > This patch introduce cdev_device_add and cdev_device_del which > replaces a common pattern including setting the kobj parent, calling > cdev_add and then calling device_add. It also introduces cdev_set_parent > for the few cases that set the kobject parent without using device_add. > > [1] https://lkml.org/lkml/2017/2/13/700 > [2] https://lkml.org/lkml/2017/2/10/370 > > Signed-off-by: Logan Gunthorpe <log...@deltatee.com> > Signed-off-by: Dan Williams <dan.j.willi...@intel.com> Reviewed-by: Alexandre Belloni <alexandre.bell...@free-electrons.com>
-- Alexandre Belloni, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
