netdev - Re: [PATCH net-next v3 2/2] 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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 19 Jul 2023 12:17:47 +0100
From: Donald Hunter <donald.hunter@...il.com>
To: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>
Cc: kuba@...nel.org, netdev@...r.kernel.org, davem@...emloft.net, 
	pabeni@...hat.com, edumazet@...gle.com
Subject: Re: [PATCH net-next v3 2/2] tools: ynl-gen: fix parse multi-attr enum attribute

On Tue, 18 Jul 2023 at 17:24, Arkadiusz Kubalewski
<arkadiusz.kubalewski@...el.com> wrote:
>
>  tools/net/ynl/lib/ynl.py | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
> index 5db7d47067f9..671ef4b5eaa6 100644
> --- a/tools/net/ynl/lib/ynl.py
> +++ b/tools/net/ynl/lib/ynl.py
> @@ -135,7 +135,7 @@ class NlAttr:
>          format = self.get_format(type)
>          return [ x[0] for x in format.iter_unpack(self.raw) ]
>
> -    def as_struct(self, members):
> +    def as_struct(self, members, attr_spec):

No need to pass attr_spec - it's the spec for the struct, not for the members.

>          value = dict()
>          offset = 0
>          for m in members:
> @@ -147,6 +147,9 @@ class NlAttr:
>                  format = self.get_format(m.type, m.byte_order)
>                  [ decoded ] = format.unpack_from(self.raw, offset)
>                  offset += format.size
> +
> +            if m.enum:
> +                decoded = self._decode_enum(decoded, attr_spec)

_decode_enum is not a method on NlAttr so I'm pretty sure this will
fail. Looks like we need to move _decode_enum() into NlAttr?

The second param to _decode_enum should be 'm' which is the attr spec
for the member.

>              if m.display_hint:
>                  decoded = self.formatted_string(decoded, m.display_hint)
>              value[m.name] = decoded
> @@ -417,8 +420,7 @@ class YnlFamily(SpecFamily):
>          pad = b'\x00' * ((4 - len(attr_payload) % 4) % 4)
>          return struct.pack('HH', len(attr_payload) + 4, nl_type) + attr_payload + pad
>
> -    def _decode_enum(self, rsp, attr_spec):
> -        raw = rsp[attr_spec['name']]
> +    def _decode_enum(self, raw, attr_spec):
>          enum = self.consts[attr_spec['enum']]
>          if 'enum-as-flags' in attr_spec and attr_spec['enum-as-flags']:
>              i = attr_spec.get('value-start', 0)
> @@ -430,15 +432,12 @@ class YnlFamily(SpecFamily):
>                  i += 1
>          else:
>              value = enum.entries_by_val[raw].name
> -        rsp[attr_spec['name']] = value
> +        return value
>
>      def _decode_binary(self, attr, attr_spec):
>          if attr_spec.struct_name:
>              members = self.consts[attr_spec.struct_name]
> -            decoded = attr.as_struct(members)
> -            for m in members:
> -                if m.enum:
> -                    self._decode_enum(decoded, m)
> +            decoded = attr.as_struct(members, attr_spec)
>          elif attr_spec.sub_type:
>              decoded = attr.as_c_array(attr_spec.sub_type)
>          else:
> @@ -466,6 +465,9 @@ class YnlFamily(SpecFamily):
>              else:
>                  raise Exception(f'Unknown {attr_spec["type"]} with name {attr_spec["name"]}')
>
> +            if 'enum' in attr_spec:
> +                decoded = self._decode_enum(int.from_bytes(attr.raw, "big"), attr_spec)

As Jakub said, this should just be self._decode_enum(decoded, attr_spec)

> +
>              if not attr_spec.is_multi:
>                  rsp[attr_spec['name']] = decoded
>              elif attr_spec.name in rsp:
> @@ -473,8 +475,6 @@ class YnlFamily(SpecFamily):
>              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):
> --
> 2.38.1