Subject: Re: [PATCH net-next v2 10/10] ethtool: netlink: always pass genl_info to .prepare_data

From: Jakub Kicinski <[email-masked]>
Date: Fri, 11 Aug 2023 15:24:40 -0700

On Fri, 11 Aug 2023 09:29:58 +0200 Jiri Pirko wrote:
> >> 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.