linux-kernel - Re: [PATCH] of: base: Change logic in of_alias_get_alias

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] [day] [month] [year] [list]

Message-ID: <2bbc70c9-21b0-dfa2-ddea-554afb2c4f89@xilinx.com>
Date:   Tue, 9 Oct 2018 09:18:53 +0200
From:   Michal Simek <michal.simek@...inx.com>
To:     Rob Herring <robh+dt@...nel.org>,
        Michal Simek <michal.simek@...inx.com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Michal Simek <monstr@...str.eu>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        <devicetree@...r.kernel.org>, Jiri Slaby <jslaby@...e.com>,
        "open list:SERIAL DRIVERS" <linux-serial@...r.kernel.org>,
        Frank Rowand <frowand.list@...il.com>,
        "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH] of: base: Change logic in of_alias_get_alias_list()

On 8.10.2018 17:07, Rob Herring wrote:
> On Mon, Oct 8, 2018 at 7:19 AM Michal Simek <michal.simek@...inx.com> wrote:
>>
>> Check compatible string first before setting up bit in bitmap to also
>> cover cases that allocated bitfield is not big enough.
>> Show warning about it but let driver to continue to work with allocated
>> bitfield to keep at least some devices (included console which
>> is commonly close to serial0) to work.
>>
>> Fixes: b1078c355d76 ("of: base: Introduce of_alias_get_alias_list() to check alias IDs")
>> Fixes: ae1cca3fa347 ("serial: uartps: Change uart ID port allocation")
>> Signed-off-by: Michal Simek <michal.simek@...inx.com>
>> ---
>>
>> I have looked at it and I don't think there should be necessary to
>> report error immediately back with partially initialized bitfield.
>> The reason is that still there could be a console device which is most
>> likely below that max limit and it is worth to return at least that nbits
>> properly filled.
>> It will also enable cases that you can still continue to use aliases
>> higher then fields prepared for devices without alias.
> 
> Seems reasonable. Plus if you had a new dtb which added an alias
> greater than what the OS version supports, you would break that
> system.

I was checking that with our uart ps driver before I send this patch and
I found out that there is no reason not to support these cases.
Driver will simply find out ids which are free and ready for devices
which don't have alias.

> 
>>
>> To be fixed patches are present in tty-next branch.
>>
>> ---
>>  drivers/of/base.c                  | 22 ++++++++++++----------
>>  drivers/tty/serial/xilinx_uartps.c |  2 +-
>>  2 files changed, 13 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index 908de45f966b..0b9611e196d1 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -1953,13 +1953,15 @@ int of_alias_get_id(struct device_node *np, const char *stem)
>>   * The function travels the lookup table to record alias ids for the given
>>   * device match structures and alias stem.
>>   *
>> - * Return:     0 or -ENOSYS when !CONFIG_OF
>> + * Return:     0 or -ENOSYS when !CONFIG_OF or
>> + *             -EINVAL if alias ID is greater then allocated nbits
> 
> I think EOVERFLOW or ERANGE would be better as those are less common
> and I take EINVAL as the caller made an error.

That's a good point I was also thinking if EINVAL is good reaction on
this case. I will use EOVERFLOW if there is no issue with it.

Thanks,
Michal