linux-kernel - Re: [PATCH 0/4] treewide: add missing put

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAErSpo4dJFsK+Wcfz8PrH=Ras343nLS=XgQFbEr_KCgFVED52Q@mail.gmail.com>
Date:	Fri, 13 Dec 2013 13:42:05 -0700
From:	Bjorn Helgaas <bhelgaas@...gle.com>
To:	Levente Kurusa <levex@...ux.com>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH 0/4] treewide: add missing put_device calls

[+cc Greg]

On Fri, Dec 13, 2013 at 12:22 PM, Levente Kurusa <levex@...ux.com> wrote:
> Hi,
>
> This is just the beginning of patchset-set that aims to fix possible
> problems caused by not calling put_device() if device_register() fails.
>
> The root cause for the need to call put_device() is that the underlying
> kobject still has a reference count of 1. Thus, device.release() will not
> be called and the device will just sit there waiting for a put_device().
> Adding the put_device() also removes the need for the call to kfree() as most
> release functions already call kfree() on the container of the device.
>
> While these have not been experienced, they are potential issues and thus
> they need to be fixed. Also, they are a few more files that have the same
> kind of issue, those will be fixed if these are accepted.

Thanks for doing this.  This is the sort of mistake that just gets
copied everywhere, so fixing the examples in the tree will help
prevent the problem from spreading more.

I don't know if there's really value in having device_register()
return an error but rely on the caller to do the put_device().  Are
there cases where the caller still needs the struct device even if
device_register() fails?  E.g., could we do something like this
instead (I know some callers would also require corresponding changes
to avoid double puts):

--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1139,8 +1139,13 @@ EXPORT_SYMBOL_GPL(device_add);
  */
 int device_register(struct device *dev)
 {
+       int ret;
+
        device_initialize(dev);
-       return device_add(dev);
+       ret = device_add(dev);
+       if (ret)
+               put_device(dev);
+       return ret;
 }
 EXPORT_SYMBOL_GPL(device_register);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/