[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-kernel
Subject:    Re: linux-next: dock_link_device is oopsy
From:       Holger Macht <holger () homac ! de>
Date:       2012-02-18 19:57:23
Message-ID: 20120218195722.GA2590 () homac ! suse ! de
[Download RAW message or body]

On Sa 18. Feb - 10:46:04, Hugh Dickins wrote:
> On Sat, 18 Feb 2012, Holger Macht wrote:
> > How about that one?
> 
> It's more broken than that.  Here's my attempt.  It boots on the
> systems with dock_station_count 0, and it boots on my laptop with
> dock_station_count 2; but I don't actually have any docking station,
> so it still doesn't test very much (dock is 0 after the loop).

Well, there doesn't have to actually exist a physical dock station (or
bay device) for dock_station_count to be > 0. It just tells that the
ACPI objects are present and thus the system is capable of it.

So does this function actually also break on your laptop and you're
getting the oops there, too?

> I have no idea if what goes on in the loop is correct, but it looks
> to me as if (as predicted) there's further breakage, that it would
> have been writing beyond the end of what it allocated if I did have
> a docking station.
> 
> Hugh
> 
> [PATCH] dock: fix bootup oops and other dock_link breakage
> 
> dock_link_device() and dock_unlink_device() should bail out early
> to avoid oops on zero-length kmalloc() when dock_station_count is 0.
> 
> But isn't there an off-by-one in that kmalloc() length anyway?
> An extra NULL appended at the end suggests so.
> 
> Rework the ordering with gotos on failure to fix several issues.
> 
> And presumably dock_unlink_device() should be presenting the same
> interface as dock_link_device(), with NULL returned when none found.
> 
> Signed-off-by: Hugh Dickins <hughd@google.com>

Fine with me.

Thanks,
 Holger
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
[prev in list] [next in list] [prev in thread] [next in thread]