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] [day] [month] [year] [list]
Message-ID: <DM6PR11MB4657642B8DDA56A8EE5CE24D9B882@DM6PR11MB4657.namprd11.prod.outlook.com>
Date: Fri, 23 Aug 2024 21:39:48 +0000
From: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>
To: Donald Hunter <donald.hunter@...il.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "kuba@...nel.org"
	<kuba@...nel.org>, "davem@...emloft.net" <davem@...emloft.net>,
	"edumazet@...gle.com" <edumazet@...gle.com>, "pabeni@...hat.com"
	<pabeni@...hat.com>, "jiri@...nulli.us" <jiri@...nulli.us>, "Keller, Jacob E"
	<jacob.e.keller@...el.com>, "liuhangbin@...il.com" <liuhangbin@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [RFC PATCH net] tools/net/ynl: fix cli.py --subscribe feature

>From: Donald Hunter <donald.hunter@...il.com>
>Sent: Friday, August 23, 2024 11:23 PM
>
>On Fri, 23 Aug 2024 at 20:31, Kubalewski, Arkadiusz
><arkadiusz.kubalewski@...el.com> wrote:
>>
>> >The problem is that we are trying to look up the op before calling
>> >nlproto.decode(...) but it wants to know the op to check if it has a
>> >fixed header.
>> >
>> >I think the fix would be to change NetlinkProtocol.decode() to perform
>> >the op lookup, if necessary, after it has called self._decode() to
>> >unpack the GenlMsg.
>> >
>> >How about changing NetlinkProtocol.decode() to be:
>> >
>> >def decode(self, ynl, nl_msg, op, ops_by_value):
>> >    msg = self._decode(nl_msg)
>> >    if op is None:
>> >        op = ops_by_value[msg.cmd()]
>> >    ...
>> >
>> >The main loop can call it like this:
>> >
>> >nlproto.decode(self, nl_msg, op, self.rsp_by_value)
>> >
>> >and check_ntf() can call it like this:
>> >
>> >nlproto.decode(self, nl_msg, None, self.rsp_by_value)
>> >
>>
>> Yes, again, this seems much better, I will prepare new patch and send
>> the non-RFC version soon.
>>
>> Thanks for your help!
>> Arkadiusz
>
>I tried the changes locally to check it would work and ended up with
>the following patch. Can you verify that it fixes the issue you had?
>
>diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
>index d42c1d605969..311542a8aa24 100644
>--- a/tools/net/ynl/lib/ynl.py
>+++ b/tools/net/ynl/lib/ynl.py
>@@ -386,8 +386,10 @@ class NetlinkProtocol:
>     def _decode(self, nl_msg):
>         return nl_msg
>
>-    def decode(self, ynl, nl_msg, op):
>+    def decode(self, ynl, nl_msg, op, ops_by_value):
>         msg = self._decode(nl_msg)
>+        if op is None:
>+            op = ops_by_value[msg.cmd()]
>         fixed_header_size = ynl._struct_size(op.fixed_header)
>         msg.raw_attrs = NlAttrs(msg.raw, fixed_header_size)
>         return msg
>@@ -921,8 +923,7 @@ class YnlFamily(SpecFamily):
>                     print("Netlink done while checking for ntf!?")
>                     continue
>
>-                op = self.rsp_by_value[nl_msg.cmd()]
>-                decoded = self.nlproto.decode(self, nl_msg, op)
>+                decoded = self.nlproto.decode(self, nl_msg, None,
>self.rsp_by_value)
>                 if decoded.cmd() not in self.async_msg_ids:
>                     print("Unexpected msg id done while checking for
>ntf", decoded)
>                     continue
>@@ -980,7 +981,7 @@ class YnlFamily(SpecFamily):
>                     if nl_msg.extack:
>                         self._decode_extack(req_msg, op, nl_msg.extack)
>                 else:
>-                    op = self.rsp_by_value[nl_msg.cmd()]
>+                    op = None
>                     req_flags = []
>
>                 if nl_msg.error:
>@@ -1004,7 +1005,7 @@ class YnlFamily(SpecFamily):
>                     done = len(reqs_by_seq) == 0
>                     break
>
>-                decoded = self.nlproto.decode(self, nl_msg, op)
>+                decoded = self.nlproto.decode(self, nl_msg, op,
>self.rsp_by_value)
>
>                 # Check if this is a reply to our request
>                 if nl_msg.nl_seq not in reqs_by_seq or decoded.cmd()
>!= op.rsp_value:

I can confirm the notification works with above changes, but there is third
call to decode which would also need an update?
It shows up on testing messages which return extack errors.
Beware line numbers, these can differ on my ynl.py as had it modified slightly.

  File "/root/arek/linux-dpll/tools/net/ynl/lib/ynl.py", line 1040, in do
    return self._op(method, vals, flags)
  File "/root/arek/linux-dpll/tools/net/ynl/lib/ynl.py", line 1034, in _op
    ret = self._ops(ops)
  File "/root/arek/linux-dpll/tools/net/ynl/lib/ynl.py", line 982, in _ops
    self._decode_extack(req_msg, op, nl_msg.extack)
  File "/root/arek/linux-dpll/tools/net/ynl/lib/ynl.py", line 801, in _decode_extack
    msg = self.nlproto.decode(self, NlMsg(request, 0, op.attr_set), op)
TypeError: decode() missing 1 required positional argument: 'ops_by_value'
error: 1

Thank you!
Arkadiusz

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ