Re: [PATCH] vme: remove unneeded kfree
On Thu, 2018-09-06 at 22:04 -0700, Linus Torvalds wrote: > On Thu, Sep 6, 2018 at 1:51 AM Ding Xiang > wrote: > > > > put_device will call vme_dev_release to free vdev, kfree is > > unnecessary here. > > That does seem to be the case. I think "unnecessary" is overly kind, > it does seem to be a double free. > > Looks like the issue was introduced back in 2013 by commit > def1820d25fa ("vme: add missing put_device() after device_register() > fails"). > > It seems you should *either* kfree() the vdev, _or_ do put_device(), > but doing both seems wrong. > > I presume the device_register() has never failed, and this being > vme-only I'm guessing there isn't a vibrant testing community. > I think that is also overly kind :-) I currently lack access to suitable hardware to test fully myself and I need to find some time to (re)implement some automated testing, after I lost access to the bits I had when I left a previous employer. That and see if I can get access to some hardware again... Manohar, do you still have access/interest in the VME stuff? You've been very quiet for a long time now. Martyn
Re: [PATCH] vme: remove unneeded kfree
On Thu, 2018-09-06 at 22:04 -0700, Linus Torvalds wrote: > On Thu, Sep 6, 2018 at 1:51 AM Ding Xiang > wrote: > > > > put_device will call vme_dev_release to free vdev, kfree is > > unnecessary here. > > That does seem to be the case. I think "unnecessary" is overly kind, > it does seem to be a double free. > > Looks like the issue was introduced back in 2013 by commit > def1820d25fa ("vme: add missing put_device() after device_register() > fails"). > > It seems you should *either* kfree() the vdev, _or_ do put_device(), > but doing both seems wrong. > > I presume the device_register() has never failed, and this being > vme-only I'm guessing there isn't a vibrant testing community. > I think that is also overly kind :-) I currently lack access to suitable hardware to test fully myself and I need to find some time to (re)implement some automated testing, after I lost access to the bits I had when I left a previous employer. That and see if I can get access to some hardware again... Manohar, do you still have access/interest in the VME stuff? You've been very quiet for a long time now. Martyn
Re: [PATCH] vme: remove unneeded kfree
On Thu, Sep 06, 2018 at 10:04:49PM -0700, Linus Torvalds wrote: > On Thu, Sep 6, 2018 at 1:51 AM Ding Xiang > wrote: > > > > put_device will call vme_dev_release to free vdev, kfree is > > unnecessary here. > > That does seem to be the case. I think "unnecessary" is overly kind, > it does seem to be a double free. > > Looks like the issue was introduced back in 2013 by commit > def1820d25fa ("vme: add missing put_device() after device_register() > fails"). > > It seems you should *either* kfree() the vdev, _or_ do put_device(), > but doing both seems wrong. You should only ever call put_device() after you have created the structure, the documentation should say that somewhere... > I presume the device_register() has never failed, and this being > vme-only I'm guessing there isn't a vibrant testing community. > > Greg? It's the correct fix, I'll queue it up soon, thanks. greg k-h
Re: [PATCH] vme: remove unneeded kfree
On Thu, Sep 06, 2018 at 10:04:49PM -0700, Linus Torvalds wrote: > On Thu, Sep 6, 2018 at 1:51 AM Ding Xiang > wrote: > > > > put_device will call vme_dev_release to free vdev, kfree is > > unnecessary here. > > That does seem to be the case. I think "unnecessary" is overly kind, > it does seem to be a double free. > > Looks like the issue was introduced back in 2013 by commit > def1820d25fa ("vme: add missing put_device() after device_register() > fails"). > > It seems you should *either* kfree() the vdev, _or_ do put_device(), > but doing both seems wrong. You should only ever call put_device() after you have created the structure, the documentation should say that somewhere... > I presume the device_register() has never failed, and this being > vme-only I'm guessing there isn't a vibrant testing community. > > Greg? It's the correct fix, I'll queue it up soon, thanks. greg k-h
Re: [PATCH] vme: remove unneeded kfree
On Thu, Sep 6, 2018 at 1:51 AM Ding Xiang wrote: > > put_device will call vme_dev_release to free vdev, kfree is > unnecessary here. That does seem to be the case. I think "unnecessary" is overly kind, it does seem to be a double free. Looks like the issue was introduced back in 2013 by commit def1820d25fa ("vme: add missing put_device() after device_register() fails"). It seems you should *either* kfree() the vdev, _or_ do put_device(), but doing both seems wrong. I presume the device_register() has never failed, and this being vme-only I'm guessing there isn't a vibrant testing community. Greg? Linus
Re: [PATCH] vme: remove unneeded kfree
On Thu, Sep 6, 2018 at 1:51 AM Ding Xiang wrote: > > put_device will call vme_dev_release to free vdev, kfree is > unnecessary here. That does seem to be the case. I think "unnecessary" is overly kind, it does seem to be a double free. Looks like the issue was introduced back in 2013 by commit def1820d25fa ("vme: add missing put_device() after device_register() fails"). It seems you should *either* kfree() the vdev, _or_ do put_device(), but doing both seems wrong. I presume the device_register() has never failed, and this being vme-only I'm guessing there isn't a vibrant testing community. Greg? Linus
[PATCH] vme: remove unneeded kfree
put_device will call vme_dev_release to free vdev, kfree is unnecessary here. Signed-off-by: Ding Xiang --- drivers/vme/vme.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c index 92500f6..520a5f9 100644 --- a/drivers/vme/vme.c +++ b/drivers/vme/vme.c @@ -1890,7 +1890,6 @@ static int __vme_register_driver_bus(struct vme_driver *drv, err_reg: put_device(>dev); - kfree(vdev); err_devalloc: list_for_each_entry_safe(vdev, tmp, >devices, drv_list) { list_del(>drv_list); -- 1.9.1
[PATCH] vme: remove unneeded kfree
put_device will call vme_dev_release to free vdev, kfree is unnecessary here. Signed-off-by: Ding Xiang --- drivers/vme/vme.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c index 92500f6..520a5f9 100644 --- a/drivers/vme/vme.c +++ b/drivers/vme/vme.c @@ -1890,7 +1890,6 @@ static int __vme_register_driver_bus(struct vme_driver *drv, err_reg: put_device(>dev); - kfree(vdev); err_devalloc: list_for_each_entry_safe(vdev, tmp, >devices, drv_list) { list_del(>drv_list); -- 1.9.1