1. 21 Mar, 2017 2 commits
    • Dan Williams's avatar
      device-dax: fix cdev leak · ed01e50a
      Dan Williams authored
      If device_add() fails, cleanup the cdev. Otherwise, we leak a kobj_map()
      with a stale device number.
      
      As Jason points out, there is a small possibility that userspace has
      opened and mapped the device in the time between cdev_add() and the
      device_add() failure. We need a new kill_dax_dev() helper to invalidate
      any established mappings.
      
      Fixes: ba09c01d ("dax: convert to the cdev api")
      Cc: <stable@vger.kernel.org>
      Reported-by: default avatarJason Gunthorpe <jgunthorpe@obsidianresearch.com>
      Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
      Signed-off-by: default avatarLogan Gunthorpe <logang@deltatee.com>
      Reviewed-by: default avatarJohannes Thumshirn <jthumshirn@suse.de>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      ed01e50a
    • 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/370Signed-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
  2. 17 Mar, 2017 38 commits