Thread (1 message) 1 message, 1 author, 2017-02-21

[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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help