Re: [PATCH] of: fix of_property_read_string

[Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On Wed, Apr 8, 2015 at 1:29 AM, Shengzhou.Liu-KZfg59tc24xl57MIdRCFDg@public.gmane.org
<Shengzhou.Liu-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
>> On Tue, Apr 7, 2015 at 5:11 AM, Shengzhou Liu
>> <Shengzhou.Liu-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
>> > In of_property_read_string function, strnlen(prop->value,
>> > prop->length) is always less or equal to prop->length, and we should
>> allow the '=='
>> > condition, so let's remove the original unreasonable condition.
>>
>> I believe we don't want to allow equal because prop->length should
>> include the \0 termination while strnlen will not.
>>
>> Rob
>
> Yes, I thought so, ideally prop->length should be assigned with strlen(value)+1,
> but unfortunately in u-boot and kernel there are too many callers that have prop->length assigned
> with strlen(value) instead of strlen(value)+1, in practice, we can allow equal.

No one should touch prop->length and it comes from the DTB directly.
It doesn't come from strlen.

> For example, in of_property_read_string_helper function, following two lines had been removed by commit 6faa2909871d8937.

I don't have that commit in my tree.

> -       if (strnlen(prop->value, prop->length) >= prop->length)
> -               return -EILSEQ;
>
> So allowing equal should be acceptable, because in this function, the equal can't prevent stack overflow issue.

What stack overflow issue?

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html