Skip to content
  • Logan Gunthorpe's avatar
    chardev: add helper function to register char devs with a struct device · 233ed09d
    Logan Gunthorpe authored
    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:
    
    2f0157f1 char_dev: pin parent kobject
    
    and the first instance of the fix was then done in the input subsystem
    in the following commit:
    
    4a215aad 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:
    
    0d5b7dae iio: Prevent race between IIO chardev opening and IIO device
    (by Lars-Peter Clausen in 2013)
    
    ba0ef854 tpm: Fix initialization of the cdev
    (by Jason Gunthorpe in 2015)
    
    5b28dde5 [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: default avatarLogan Gunthorpe <logang@deltatee.com>
    Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
    Reviewed-by: default avatarHans Verkuil <hans.verkuil@cisco.com>
    Reviewed-by: default avatarAlexandre Belloni <alexandre.belloni@free-electrons.com>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    233ed09d