netdev - Re: [patch net-next] tools: ynl: fix attr_space variable to exist even if processing unknown 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] [day] [month] [year] [list]

Message-ID: <Zc4DHW05LcGy9Ryw@nanopsycho>
Date: Thu, 15 Feb 2024 13:27:09 +0100
From: Jiri Pirko <jiri@...nulli.us>
To: Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org, kuba@...nel.org, davem@...emloft.net,
	edumazet@...gle.com, donald.hunter@...il.com
Subject: Re: [patch net-next] tools: ynl: fix attr_space variable to exist
 even if processing unknown attribute

Thu, Feb 15, 2024 at 01:13:45PM CET, jiri@...nulli.us wrote:
>Thu, Feb 15, 2024 at 11:59:08AM CET, pabeni@...hat.com wrote:
>>On Tue, 2024-02-13 at 08:04 +0100, Jiri Pirko wrote:
>>> From: Jiri Pirko <jiri@...dia.com>
>>> 
>>> If message contains unknown attribute and user passes
>>> "--process-unknown" command line option, _decode() gets called with space
>>> arg set to None. In that case, attr_space variable is not initialized
>>> used which leads to following trace:
>>> 
>>> Traceback (most recent call last):
>>>   File "./tools/net/ynl/cli.py", line 77, in <module>
>>>     main()
>>>   File "./tools/net/ynl/cli.py", line 68, in main
>>>     reply = ynl.dump(args.dump, attrs)
>>>             ^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>   File "tools/net/ynl/lib/ynl.py", line 909, in dump
>>>     return self._op(method, vals, [], dump=True)
>>>            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>   File "tools/net/ynl/lib/ynl.py", line 894, in _op
>>>     rsp_msg = self._decode(decoded.raw_attrs, op.attr_set.name)
>>>               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>   File "tools/net/ynl/lib/ynl.py", line 639, in _decode
>>>     self._rsp_add(rsp, attr_name, None, self._decode_unknown(attr))
>>>                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>   File "tools/net/ynl/lib/ynl.py", line 569, in _decode_unknown
>>>     return self._decode(NlAttrs(attr.raw), None)
>>>            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>   File "tools/net/ynl/lib/ynl.py", line 630, in _decode
>>>     search_attrs = SpaceAttrs(attr_space, rsp, outer_attrs)
>>>                               ^^^^^^^^^^
>>> UnboundLocalError: cannot access local variable 'attr_space' where it is not associated with a value
>>> 
>>> Fix this by setting attr_space to None in case space is arg None.
>>> 
>>> Fixes: bf8b832374fb ("tools/net/ynl: Support sub-messages in nested attribute spaces")
>>> Signed-off-by: Jiri Pirko <jiri@...dia.com>
>>> ---
>>>  tools/net/ynl/lib/ynl.py | 6 ++++--
>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
>>> index 03c7ca6aaae9..b16d24b7e288 100644
>>> --- a/tools/net/ynl/lib/ynl.py
>>> +++ b/tools/net/ynl/lib/ynl.py
>>> @@ -588,10 +588,12 @@ class YnlFamily(SpecFamily):
>>>          revalue = search_attrs.lookup(selector)turn decoded
>>>  
>>>      def _decode(self, attrs, space, outer_attrs = None):
>>> +        rsp = dict()
>>>          if space:
>>>              attr_space = self.attr_sets[space]
>>> -        rsp = dict()
>>> -        search_attrs = SpaceAttrs(attr_space, rsp, outer_attrs)
>>> +            search_attrs = SpaceAttrs(attr_space, rsp, outer_attrs)
>>> +        else:
>>> +            search_attrs = None
>>
>>It looks like that later-on the code could call self._decode_sub_msg()
>>-> self._resolve_selector() with search_attrs == None, and the latter
>>will unconditionally do:
>>
>>	value = search_attrs.lookup(selector)
>
>How exactly you can reach this? You won't get past:
>            try:
>                attr_spec = attr_space.attrs_by_val[attr.type]
>            except (KeyError, UnboundLocalError):
>                if not self.process_unknown:
>                    raise Exception(f"Space '{space}' has no attribute with value '{attr.type}'")
>                attr_name = f"UnknownAttr({attr.type})"
>                self._rsp_add(rsp, attr_name, None, self._decode_unknown(attr))
>                continue

Okay, I can just avoid setting search_attrs = None, as it it not used
anyway to avoid the confusion. Sending v2.

>
>
>
>>
>>I think we need to explicitly handle the None value there.
>>
>>Thanks,
>>
>>Paolo
>>