Re: linux-next: dock_link_device is oopsy

[Holger Macht <holger@homac.de>]

On Sa 18. Feb - 21:37:18, Hillf Danton wrote:
> On Sat, Feb 18, 2012 at 9:26 PM, Holger Macht <holger@homac.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@homac.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