Messages in this thread | | | From | Logan Gunthorpe <> | Date | Tue, 22 May 2018 10:56:51 -0600 | Subject | Re: [PATCH] ntb_transport: use put_device() instead of kfree() |
| |
On 21/05/18 06:48 PM, Jon Mason wrote: > On Fri, Mar 09, 2018 at 04:03:24PM +0530, Arvind Yadav wrote: >> Never directly free @dev after calling device_register(), even >> if it returned an error! Always use put_device() to give up the >> reference initialized. >> >> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com> >> --- >> drivers/ntb/ntb_transport.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c >> index 9878c48..8182a3a 100644 >> --- a/drivers/ntb/ntb_transport.c >> +++ b/drivers/ntb/ntb_transport.c >> @@ -393,7 +393,7 @@ int ntb_transport_register_client_dev(char *device_name) >> >> rc = device_register(dev); >> if (rc) { >> - kfree(client_dev); >> + put_device(dev); > > Now we are leaking client_dev, which is bigger than just dev. I think > we are going to need both now.
No, when put_device is called, ntb_transport_client_release() will be called which then kfree's the structure there. This is the preferred way of freeing anything that's reference counted (as all struct devices are). See [1].
Though, if I remember correctly, the NTB tree breaks this rule a lot. ie. the fact that ntb_dev_release() doesn't actually free the underlying structure has always irked me a bit because it forces the calling drivers to break this rule. This means the reference counting on the NTB devices is broken so if someone ever uses get_device() on an struct ntb_dev and doesn't put it back before the driver is unbound, there will be a use after free bug. As far as I know though, at this time this isn't done.
Logan
[1] https://elixir.bootlin.com/linux/v4.17-rc6/source/drivers/base/core.c#L1931
| |