netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch net-next] tools: ynl: fix attr_space variable to exist even if processing unknown attribute
@ 2024-02-13  7:04 Jiri Pirko
  2024-02-15 10:59 ` Paolo Abeni
  0 siblings, 1 reply; 4+ messages in thread
From: Jiri Pirko @ 2024-02-13  7:04 UTC (permalink / raw)
  To: netdev; +Cc: kuba, pabeni, davem, edumazet, donald.hunter

From: Jiri Pirko <jiri@nvidia.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@nvidia.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):
         return 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
 
         for attr in attrs:
             try:
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [patch net-next] tools: ynl: fix attr_space variable to exist even if processing unknown attribute
  2024-02-13  7:04 [patch net-next] tools: ynl: fix attr_space variable to exist even if processing unknown attribute Jiri Pirko
@ 2024-02-15 10:59 ` Paolo Abeni
  2024-02-15 12:13   ` Jiri Pirko
  0 siblings, 1 reply; 4+ messages in thread
From: Paolo Abeni @ 2024-02-15 10:59 UTC (permalink / raw)
  To: Jiri Pirko, netdev; +Cc: kuba, davem, edumazet, donald.hunter

On Tue, 2024-02-13 at 08:04 +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.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@nvidia.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)

I think we need to explicitly handle the None value there.

Thanks,

Paolo


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [patch net-next] tools: ynl: fix attr_space variable to exist even if processing unknown attribute
  2024-02-15 10:59 ` Paolo Abeni
@ 2024-02-15 12:13   ` Jiri Pirko
  2024-02-15 12:27     ` Jiri Pirko
  0 siblings, 1 reply; 4+ messages in thread
From: Jiri Pirko @ 2024-02-15 12:13 UTC (permalink / raw)
  To: Paolo Abeni; +Cc: netdev, kuba, davem, edumazet, donald.hunter

Thu, Feb 15, 2024 at 11:59:08AM CET, pabeni@redhat.com wrote:
>On Tue, 2024-02-13 at 08:04 +0100, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@nvidia.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@nvidia.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



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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [patch net-next] tools: ynl: fix attr_space variable to exist even if processing unknown attribute
  2024-02-15 12:13   ` Jiri Pirko
@ 2024-02-15 12:27     ` Jiri Pirko
  0 siblings, 0 replies; 4+ messages in thread
From: Jiri Pirko @ 2024-02-15 12:27 UTC (permalink / raw)
  To: Paolo Abeni; +Cc: netdev, kuba, davem, edumazet, donald.hunter

Thu, Feb 15, 2024 at 01:13:45PM CET, jiri@resnulli.us wrote:
>Thu, Feb 15, 2024 at 11:59:08AM CET, pabeni@redhat.com wrote:
>>On Tue, 2024-02-13 at 08:04 +0100, Jiri Pirko wrote:
>>> From: Jiri Pirko <jiri@nvidia.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@nvidia.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
>>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-02-15 12:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-13  7:04 [patch net-next] tools: ynl: fix attr_space variable to exist even if processing unknown attribute Jiri Pirko
2024-02-15 10:59 ` Paolo Abeni
2024-02-15 12:13   ` Jiri Pirko
2024-02-15 12:27     ` Jiri Pirko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).