From: Jakub Kicinski <kuba@kernel.org>
To: Jiri Pirko <jiri@resnulli.us>
Cc: Michal Kubecek <mkubecek@suse.cz>,
davem@davemloft.net, netdev@vger.kernel.org, edumazet@google.com,
pabeni@redhat.com, johannes@sipsolutions.net,
Vladimir Oltean <vladimir.oltean@nxp.com>,
gal@nvidia.com, tariqt@nvidia.com, lucien.xin@gmail.com,
f.fainelli@gmail.com, andrew@lunn.ch, simon.horman@corigine.com,
linux@rempel-privat.de
Subject: Re: [PATCH net-next v2 10/10] ethtool: netlink: always pass genl_info to .prepare_data
Date: Fri, 11 Aug 2023 15:24:40 -0700 [thread overview]
Message-ID: <20230811152440.516c6bc2@kernel.org> (raw)
In-Reply-To: <ZNXjdj3edS1Up3Mt@nanopsycho>
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.
prev parent reply other threads:[~2023-08-11 22:24 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-10 23:38 [PATCH net-next v2 00/10] genetlink: provide struct genl_info to dumps Jakub Kicinski
2023-08-10 23:38 ` [PATCH net-next v2 01/10] genetlink: push conditional locking into dumpit/done Jakub Kicinski
2023-08-10 23:38 ` [PATCH net-next v2 02/10] genetlink: make genl_info->nlhdr const Jakub Kicinski
2023-08-10 23:38 ` [PATCH net-next v2 03/10] genetlink: remove userhdr from struct genl_info Jakub Kicinski
2023-08-10 23:38 ` [PATCH net-next v2 04/10] genetlink: add struct genl_info to struct genl_dumpit_info Jakub Kicinski
2023-08-10 23:38 ` [PATCH net-next v2 05/10] genetlink: use attrs from struct genl_info Jakub Kicinski
2023-08-10 23:38 ` [PATCH net-next v2 06/10] genetlink: add a family pointer to " Jakub Kicinski
2023-08-11 6:28 ` Jiri Pirko
2023-08-10 23:38 ` [PATCH net-next v2 07/10] genetlink: add genlmsg_iput() API Jakub Kicinski
2023-08-11 6:44 ` Jiri Pirko
2023-08-10 23:38 ` [PATCH net-next v2 08/10] netdev-genl: use struct genl_info for reply construction Jakub Kicinski
2023-08-11 6:44 ` Jiri Pirko
2023-08-10 23:38 ` [PATCH net-next v2 09/10] ethtool: netlink: simplify arguments to ethnl_default_parse() Jakub Kicinski
2023-08-10 23:38 ` [PATCH net-next v2 10/10] ethtool: netlink: always pass genl_info to .prepare_data Jakub Kicinski
2023-08-11 6:42 ` Jiri Pirko
2023-08-11 7:13 ` Michal Kubecek
2023-08-11 7:29 ` Jiri Pirko
2023-08-11 22:24 ` Jakub Kicinski [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230811152440.516c6bc2@kernel.org \
--to=kuba@kernel.org \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=f.fainelli@gmail.com \
--cc=gal@nvidia.com \
--cc=jiri@resnulli.us \
--cc=johannes@sipsolutions.net \
--cc=linux@rempel-privat.de \
--cc=lucien.xin@gmail.com \
--cc=mkubecek@suse.cz \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=simon.horman@corigine.com \
--cc=tariqt@nvidia.com \
--cc=vladimir.oltean@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).