On Fri, 11 Aug 2023 09:29:58 +0200 Jiri Pirko wrote:

quoted

quoted

Anyway, the genl_info_is_ntf() itself seems a bit odd to me. The only
user is here and I doubt there ever going to be any other. This
conditional per-op attr fill seems a bit odd.

Can't you handle this in side ethtool somehow? IDK :/  

I don't think so. The point here is that notification can be seen by any
unprivileged process so as long as we agree that those should not see
the wake up passwords, we must not include the password in them. While
ethtool could certanly drop the password from its output, any other
utility parsing the notifications (or even patched ethtool) could still
show it to anyone.  

Yeah, the question is, if it is a good design to have one CMD type
to conditionally send sensitive data. I would argue that sensitive data
could be sent over separate CMD with no notifier for it.

Good catch!

Hopefully we can address that separately (I mean someone who cares can
send a patch? :)). We had multiple people get surprised by info being
NULL I think the value of the other changes outweighs resolving this
little oddity. I'm going to send a v3 with the bug fixed later.

On the existence of genl_info_is_ntf(), I would rather keep it.
I'm a bit worried someone else will need to know at some point and
will do it based on contents of info directly, which will make
future refactoring risky.