linux-kernel - Re: linux-next: dock_link

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: <20120218140449.GA2558@homac.suse.de>
Date:	Sat, 18 Feb 2012 15:04:49 +0100
From:	Holger Macht <holger@...ac.de>
To:	Hillf Danton <dhillf@...il.com>
Cc:	Hugh Dickins <hughd@...gle.com>, Matthew Garrett <mjg@...hat.com>,
	Jeff Garzik <jgarzik@...hat.com>,
	Stephen Rothwell <sfr@...b.auug.org.au>,
	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: linux-next: dock_link_device is oopsy

On Sa 18. Feb - 21:37:18, Hillf Danton wrote:
> On Sat, Feb 18, 2012 at 9:26 PM, Holger Macht <holger@...ac.de> wrote:
> Hi  Holger
> 
> Lets check the following snippet from what you posted,
> 
> > @@ -320,12 +324,17 @@ EXPORT_SYMBOL_GPL(dock_link_device);
> >  */
> >  struct device **dock_unlink_device(acpi_handle handle)
> >  {
> > -       struct device *dev = acpi_get_physical_device(handle);
> > +       struct device *dev;
> >        struct dock_station *dock_station;
> >        int dock = 0;
> > -       struct device **devices =
> > -               kmalloc(dock_station_count * sizeof(struct device *),
> > -                       GFP_KERNEL);
> > +       struct device **devices;
> > +
> > +       if (!dock_station_count)
> > +               return NULL;
> > +
> > +       dev = acpi_get_physical_device(handle);
> > +       devices = kmalloc(dock_station_count * sizeof(struct device *),
> > +                         GFP_KERNEL);
> >
> >        if (!dev)
> >                return NULL;  <=== here return without freeing the newly
>                                                allocated devices after checking
>                                                dock_station_count is not zero

How about that one?

acpi: Bail out when linking devices and there are no dock stations

If dock_station_count is zero, we allocate zero memory and don't check
this at future references. So bail out if there are actually no dock
stations. Fix some possible memory leaks on the way.

Signed-off-by: Holger Macht <holger@...ac.de>
---
 drivers/acpi/dock.c |   23 ++++++++++++++++-------
 1 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
index b5e4142..b2e8730 100644
--- a/drivers/acpi/dock.c
+++ b/drivers/acpi/dock.c
@@ -281,14 +281,15 @@ EXPORT_SYMBOL_GPL(is_dock_device);
  */
 struct device **dock_link_device(acpi_handle handle)
 {
-	struct device *dev = acpi_get_physical_device(handle);
+	struct device *dev;
 	struct dock_station *dock_station;
 	int ret, dock = 0;
 	struct device **devices;
 
-	devices = kmalloc(dock_station_count * sizeof(struct device *),
-			  GFP_KERNEL);
+	if (!dock_station_count)
+		return NULL;
 
+	dev = acpi_get_physical_device(handle);
 	if (!dev)
 		return NULL;
 
@@ -297,6 +298,9 @@ struct device **dock_link_device(acpi_handle handle)
 		return NULL;
 	}
 
+	devices = kmalloc(dock_station_count * sizeof(struct device *),
+			  GFP_KERNEL);
+
 	list_for_each_entry(dock_station, &dock_stations, sibling) {
 		if (find_dock_dependent_device(dock_station, handle)) {
 			ret = sysfs_create_link(&dock_station->dock_device->dev.kobj,
@@ -320,13 +324,15 @@ EXPORT_SYMBOL_GPL(dock_link_device);
  */
 struct device **dock_unlink_device(acpi_handle handle)
 {
-	struct device *dev = acpi_get_physical_device(handle);
+	struct device *dev;
 	struct dock_station *dock_station;
 	int dock = 0;
-	struct device **devices =
-		kmalloc(dock_station_count * sizeof(struct device *),
-			GFP_KERNEL);
+	struct device **devices;
+
+	if (!dock_station_count)
+		return NULL;
 
+	dev = acpi_get_physical_device(handle);
 	if (!dev)
 		return NULL;
 
@@ -335,6 +341,9 @@ struct device **dock_unlink_device(acpi_handle handle)
 		return NULL;
 	}
 
+	devices = kmalloc(dock_station_count * sizeof(struct device *),
+			  GFP_KERNEL);
+
 	list_for_each_entry(dock_station, &dock_stations, sibling) {
 		if (find_dock_dependent_device(dock_station, handle)) {
 			sysfs_remove_link(&dock_station->dock_device->dev.kobj,
-- 
1.7.7

--
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/