netdev - Re: [RFC PATCH net] tools/net/ynl: fix cli.py --subscribe feature

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAD4GDZza0TWkzmwW_qP8GFXzr7DOGVfg-xsiXOVUyRqe47Rung@mail.gmail.com>
Date: Fri, 23 Aug 2024 22:22:37 +0100
From: Donald Hunter <donald.hunter@...il.com>
To: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.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

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: