| Message ID | 20240213070443.442910-1-jiri@resnulli.us (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] tools: ynl: fix attr_space variable to exist even if processing unknown attribute | expand |
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
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 >
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 >>
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: