linux-kernel - RE: [PATCH net-next] tools: ynl-gen: fix parse multi-attr enum attribute

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: <DM6PR11MB465780867DE4C45F977A06D39B36A@DM6PR11MB4657.namprd11.prod.outlook.com>
Date:   Wed, 12 Jul 2023 09:47:43 +0000
From:   "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "chuck.lever@...cle.com" <chuck.lever@...cle.com>
Subject: RE: [PATCH net-next] tools: ynl-gen: fix parse multi-attr enum
 attribute

>From: Jakub Kicinski <kuba@...nel.org>
>Sent: Wednesday, July 12, 2023 6:00 AM
>
>On Tue, 11 Jul 2023 11:53:23 +0200 Arkadiusz Kubalewski wrote:
>> diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index
>> 3b343d6cbbc0..553d82dd6382 100644
>> --- a/tools/net/ynl/lib/ynl.py
>> +++ b/tools/net/ynl/lib/ynl.py
>> @@ -407,7 +407,14 @@ class YnlFamily(SpecFamily):
>>                  raw >>= 1
>>                  i += 1
>>          else:
>> -            value = enum.entries_by_val[raw - i].name
>> +            if attr_spec.is_multi:
>> +                for index in range(len(raw)):
>> +                    if (type(raw[index]) == int):
>> +                        enum_name = enum.entries_by_val[raw[index] -
>>i].name
>> +                        rsp[attr_spec['name']][index] = enum_name
>> +                return
>> +            else:
>> +                value = enum.entries_by_val[raw - i].name
>>          rsp[attr_spec['name']] = value
>
>Two asks:
>
>First this function stupidly looks at value-start. Best I can tell this is
>a leftover from when enum set was an array, but potentially "indexed with
>an offset" (ie. if value start = 10, first elem would have value 11, second
>12 etc.). When we added support for sparse enums this was carried forward,
>but it's actually incorrect. entries_by_val is indexed with the real value,
>we should not subtract the start-value. So please send a patch to set i to
>0 at the start and ignore start-value here (or LMK if I should send one).
>
>Second, instead of fixing the value up here, after already putting it in
>the rsp - can we call this function to decode the enum before?
>A bit hard to explain, let me show you the diff of what I have in mind for
>the call site:
>
>diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index
>1b3a36fbb1c3..e2e8a8c5fb6b 100644
>--- a/tools/net/ynl/lib/ynl.py
>+++ b/tools/net/ynl/lib/ynl.py
>@@ -466,15 +466,15 @@ genl_family_name_to_id = None
>             else:
>                 raise Exception(f'Unknown {attr_spec["type"]} with name
>{attr_spec["name"]}')
>
>+            if 'enum' in attr_spec:
>+                decoded = self._decode_enum(rsp, attr_spec)
>+
>             if not attr_spec.is_multi:
>                 rsp[attr_spec['name']] = decoded
>             elif attr_spec.name in rsp:
>                 rsp[attr_spec.name].append(decoded)
>             else:
>                 rsp[attr_spec.name] = [decoded]
>-
>-            if 'enum' in attr_spec:
>-                self._decode_enum(rsp, attr_spec)
>         return rsp
>
>     def _decode_extack_path(self, attrs, attr_set, offset, target):
>
>Then _decode_enum() only has to ever deal with single values, and the
>caller will take care of mutli_attr like it would for any other type?

Sure, I will try to implement your proposal and send update here.

Thank you!
Arkadiusz

>--
>pw-bot: cr