linux-kernel - Re: [RESEND PATCH v3 1/2] of: base: Introduce 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <d27b381e-f091-91dc-cffe-27789583f949@xilinx.com>
Date:   Wed, 26 Sep 2018 13:01:02 +0200
From:   Michal Simek <michal.simek@...inx.com>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>,
        Michal Simek <michal.simek@...inx.com>
CC:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Michal Simek <monstr@...str.eu>,
        Greg KH <gregkh@...uxfoundation.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, Rob Herring <robh+dt@...nel.org>,
        Frank Rowand <frowand.list@...il.com>
Subject: Re: [RESEND PATCH v3 1/2] of: base: Introduce
 of_alias_get_alias_list() to check alias IDs

On 24.9.2018 09:41, Geert Uytterhoeven wrote:
> Hi Michal,
> 
> On Thu, Sep 20, 2018 at 1:42 PM Michal Simek <michal.simek@...inx.com> wrote:
>> The function travels the lookup table to record alias ids for the given
>> device match structures and alias stem.
>> This function will be used by serial drivers to check if requested alias
>> is allocated or free to use.
>>
>> Signed-off-by: Michal Simek <michal.simek@...inx.com>
>> Reviewed-by: Rob Herring <robh@...nel.org>
> 
> I know this has already been applied, it just drew my attention.
> 
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -16,6 +16,7 @@
>>
>>  #define pr_fmt(fmt)    "OF: " fmt
>>
>> +#include <linux/bitmap.h>
>>  #include <linux/console.h>
>>  #include <linux/ctype.h>
>>  #include <linux/cpu.h>
>> @@ -1943,6 +1944,57 @@ int of_alias_get_id(struct device_node *np, const char *stem)
>>  EXPORT_SYMBOL_GPL(of_alias_get_id);
>>
>>  /**
>> + * of_alias_get_alias_list - Get alias list for the given device driver
>> + * @matches:   Array of OF device match structures to search in
>> + * @stem:      Alias stem of the given device_node
>> + * @bitmap:    Bitmap field pointer
>> + * @nbits:     Maximum number of alias ID which can be recorded it bitmap
> 
> IDs
> 
>> + *
>> + * 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
>> + */
>> +int of_alias_get_alias_list(const struct of_device_id *matches,
>> +                            const char *stem, unsigned long *bitmap,
>> +                            unsigned int nbits)
>> +{
>> +       struct alias_prop *app;
>> +
>> +       /* Zero bitmap field to make sure that all the time it is clean */
>> +       bitmap_zero(bitmap, nbits);
>> +
>> +       mutex_lock(&of_mutex);
>> +       pr_debug("%s: Looking for stem: %s\n", __func__, stem);
>> +       list_for_each_entry(app, &aliases_lookup, link) {
>> +               pr_debug("%s: stem: %s, id: %d\n",
>> +                        __func__, app->stem, app->id);
>> +
>> +               if (strcmp(app->stem, stem) != 0) {
>> +                       pr_debug("%s: stem comparison doesn't passed %s\n",
> 
> didn't pass?
> 
>> +                                __func__, app->stem);
>> +                       continue;
>> +               }
>> +
>> +               if (app->id >= nbits) {
>> +                       pr_debug("%s: ID %d greater then bitmap field %d\n",
> 
> than
> %u for unsigned int

I won't fix this now because this can be done together with one change
described below.

> 
>> +                               __func__, app->id, nbits);
>> +                       continue;
> 
> Shouldn't this return an error, if of_match_node() below would have matched?

IIRC Above check is for "serial" name.
This one is for getting XX number from alias "serialXX".
And below is checking compatible string.

You are calling this function from uartps with 32 lines limit and there
is serial33 alias poiting to uartlite (for example) then you don't want
to break this call for that.

What can be done is that compatible string is checked first and if this
passed then ids are check and error can be returned or bit sets.

Thanks,
Michal