linux-kernel - Re: drivers/of: crash on boot

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: <CAL_JsqKn1BLCiW=CBch4G8s=YheXFVouhz+JVvB6j8-4W-UK6A@mail.gmail.com>
Date:	Wed, 18 May 2016 19:23:49 -0500
From:	Rob Herring <robh@...nel.org>
To:	Rhyland Klein <rklein@...dia.com>
Cc:	Sasha Levin <sasha.levin@...cle.com>,
	Gavin Shan <gwshan@...ux.vnet.ibm.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Grant Likely <grant.likely@...aro.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: drivers/of: crash on boot

On Wed, May 18, 2016 at 4:26 PM, Rhyland Klein <rklein@...dia.com> wrote:
> On 5/18/2016 3:58 PM, Rhyland Klein wrote:
>> On 5/18/2016 3:36 PM, Rob Herring wrote:
>>> On Wed, May 18, 2016 at 10:34 AM, Sasha Levin <sasha.levin@...cle.com> wrote:
>>>> Hi Rhyland,
>>>>
>>>> I'm seeing a crash on boot that seems to have been caused by
>>>> "drivers/of: Fix depth when unflattening devicetree":
>>>>
>>>> [   61.145229] ==================================================================
>>>>
>>>> [   61.147588] BUG: KASAN: stack-out-of-bounds in unflatten_dt_nodes+0x11d2/0x1290 at addr ffff88005b30777c
>>>
>>> The following appears to fix it for me. Rhyland, please confirm.
>>>
>>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>>> index 7f38241..888ec2a 100644
>>> --- a/drivers/of/fdt.c
>>> +++ b/drivers/of/fdt.c
>>> @@ -409,7 +409,7 @@ static int unflatten_dt_nodes(const void *blob,
>>>         fpsizes[depth] = dad ? strlen(of_node_full_name(dad)) : 0;
>>>         nps[depth+1] = dad;
>>>         for (offset = 0;
>>> -            offset >= 0;
>>> +            offset >= 0, depth >= 0;
>>>              offset = fdt_next_node(blob, offset, &depth)) {
>>>                 if (WARN_ON_ONCE(depth >= FDT_MAX_DEPTH))
>>>                         continue;
>>>
>>
>> If I try that patch, i see this when compiling:
>>
>> In function ‘unflatten_dt_nodes’:
>> warning: left-hand operand of comma expression has no effect
>> [-Wunused-value]
>>       offset >= 0, depth >= 0;

Doh! However, that does make the unit test pass and I don't see a NULL ptr...

>>
>
> This patch seems to work for me. I found a bug in my original patch.
> Sasha/Rob, can you see if this works for you too:
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 0b5850027bb5..e7a8caac5b27 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -407,9 +407,9 @@ static int unflatten_dt_nodes(const void *blob,
>
>         root = dad;
>         fpsizes[depth] = dad ? strlen(of_node_full_name(dad)) : 0;
> -       nps[depth+1] = dad;
> +       nps[depth] = dad;
>         for (offset = 0;
> -            offset >= 0;
> +            offset >= 0 && depth >= 0;
>              offset = fdt_next_node(blob, offset, &depth)) {
>                 if (WARN_ON_ONCE(depth >= FDT_MAX_DEPTH))
>                         continue;

This is not work for me. I'm booting x86 with the DT unit test and
KASAN enabled. I suspect our differences are due to different data
after the end of the dtb. Also, I think there may be a bug in
fdt_next_node FDT_END handling. The "!depth" seems suspicious to me
and I think it should be "!(*depth)".

The DT overlay unit tests are also failing. Not sure if that's related.

Rob