[rtc-linux] Re: [PATCH 07/14] infiniband: utilize new device_add_cdev helper function
From: Logan Gunthorpe <logang@deltatee.com>
Date: 2017-02-21 23:54:05
Also in:
linux-fsdevel, linux-gpio, linux-input, linux-rdma, linux-scsi, nvdimm
On 21/02/17 12:09 PM, Jason Gunthorpe wrote:
On Mon, Feb 20, 2017 at 10:00:46PM -0700, Logan Gunthorpe wrote:quoted
This patch updates core/ucm.c which didn't originally use the cdev.kobj.parent with it's parent device. I did not look heavily into whether this was a bug or not, but it seems likely to me there would be a use before free.Hum, is probably safe - ib_ucm_remove_one can only happen on module unload and the cdev holds the module lock while open.
Ah, yes looks like you are correct.
Even so device_add_cdev should be used anyhow.
Agreed.
quoted
I also took a look at core/uverbs_main.c, core/user_mad.c and hw/hfi1/device.c which utilize cdev.kobj.parent but because the infiniband core seems to use kobjs internally instead of struct devices they could not be converted to use the new helper API and still directly manipulate the internals of the kobj.Yes, and hfi1 had the same use-afte-free bug when it was first submitted as well. IHMO cdev should have a helper entry for this style of use case as well.
I agree, but I'm not sure what that api should look like. Same thing but kobject_add_cdev???
quoted
diff --git a/drivers/infiniband/core/ucm.c b/drivers/infiniband/core/ucm.c index e0a995b..38ea316 100644 +++ b/drivers/infiniband/core/ucm.c@@ -1283,18 +1283,20 @@ static void ib_ucm_add_one(struct ib_device *device) set_bit(devnum, dev_map); } + device_initialize(&ucm_dev->dev);Ah, this needs more help. Once device_initialize is called put_device should be the error-unwind, can you use something more like this?
Is that true? Once device_register or device_add is called then you need to use put_device. But I didn't think that's true for device_initialize. In fact device_add is what does the first get_device so this doesn't add up to me. device_initialize only inits some structures it doesn't do anything that needs to be torn down -- at least that I'm aware of. I know the DAX code only uses put_device after device_add. [1] Logan [1] http://lxr.free-electrons.com/source/drivers/dax/dax.c#L713 -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout.