netdev - Re: [PATCH net-next v3 2/6] tools/net/ynl: Report netlink errors without stacktrace

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

Date: Thu, 7 Mar 2024 11:56:59 +0000
From: Donald Hunter <donald.hunter@...il.com>
To: Breno Leitao <leitao@...ian.org>
Cc: kuba@...nel.org, netdev@...r.kernel.org, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Paolo Abeni <pabeni@...hat.com>, Jacob Keller <jacob.e.keller@...el.com>, 
	Jiri Pirko <jiri@...nulli.us>, Stanislav Fomichev <sdf@...gle.com>, donald.hunter@...hat.com
Subject: Re: [PATCH net-next v3 2/6] tools/net/ynl: Report netlink errors
 without stacktrace

On Thu, 7 Mar 2024 at 09:31, Breno Leitao <leitao@...ian.org> wrote:
>
> Hello Donald,
>
> On Wed, Mar 06, 2024 at 11:10:42PM +0000, Donald Hunter wrote:
> > ynl does not handle NlError exceptions so they get reported like program
> > failures. Handle the NlError exceptions and report the netlink errors
> > more cleanly.
> >
> > Example now:
> >
> > Netlink error: No such file or directory
> > nl_len = 44 (28) nl_flags = 0x300 nl_type = 2
> >       error: -2       extack: {'bad-attr': '.op'}
> >
> > Example before:
> >
> > Traceback (most recent call last):
> >   File "/home/donaldh/net-next/./tools/net/ynl/cli.py", line 81, in <module>
> >     main()
> >   File "/home/donaldh/net-next/./tools/net/ynl/cli.py", line 69, in main
> >     reply = ynl.dump(args.dump, attrs)
> >             ^^^^^^^^^^^^^^^^^^^^^^^^^^
> >   File "/home/donaldh/net-next/tools/net/ynl/lib/ynl.py", line 906, in dump
> >     return self._op(method, vals, [], dump=True)
> >            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >   File "/home/donaldh/net-next/tools/net/ynl/lib/ynl.py", line 872, in _op
> >     raise NlError(nl_msg)
> > lib.ynl.NlError: Netlink error: No such file or directory
> > nl_len = 44 (28) nl_flags = 0x300 nl_type = 2
> >       error: -2       extack: {'bad-attr': '.op'}
>
> Basically this is just hidding the stack, which may make it harder for
> someone not used to the code to find the problem.
>
> Usually fatal exception is handled to make the error more meaningful,
> i.e, better than just the exception message + stack. Hidding the stack
> and exitting may make the error less meaningful.

NlError is used to report a usage error reported by netlink as opposed
to a fatal exception. My thinking here is that it is better UX to
report netlink error responses without the stack trace precisely
because they are not exceptional. An NlError is not ynl program
breakage or subsystem breakage, it's e.g. nlctrl telling you that you
requested an op that does not exist.

> On a different topic, I am wondering if we want to add type hitting for
> these python program. They make the review process easier, and the
> development a bit more structured. (Maybe that is what we expect from
> upcoming new python code in netdev?!)

It's a good suggestion. I have never used python type hints so I'll
need to learn about them. I defer to the netdev maintainers about
whether this is something they want.

> If that is where we want to go, this is *not*, by any mean, a blocker to
> this code. Maybe something we can add to our public ToDo list (CC:
> Jakub).