From: Jakub Kicinski <kuba@kernel.org>
To: Kees Cook <keescook@chromium.org>
Cc: "David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Paolo Abeni <pabeni@redhat.com>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
"Gustavo A. R. Silva" <gustavoars@kernel.org>,
netdev@vger.kernel.org, linux-hardening@vger.kernel.org,
Simon Horman <horms@kernel.org>, Jiri Pirko <jiri@resnulli.us>,
Daniel Borkmann <daniel@iogearbox.net>,
Coco Li <lixiaoyan@google.com>,
Amritha Nambiar <amritha.nambiar@intel.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] netdev: Use flexible array for trailing private bytes
Date: Thu, 29 Feb 2024 22:59:10 -0800 [thread overview]
Message-ID: <20240229225910.79e224cf@kernel.org> (raw)
In-Reply-To: <20240229213018.work.556-kees@kernel.org>
On Thu, 29 Feb 2024 13:30:22 -0800 Kees Cook wrote:
> Introduce a new struct net_device_priv that contains struct net_device
> but also accounts for the commonly trailing bytes through the "size" and
> "data" members.
I'm a bit unclear on the benefit. Perhaps I'm unaccustomed to "safe C".
> As many dummy struct net_device instances exist still,
> it is non-trivial to but this flexible array inside struct net_device
put
Non-trivial, meaning what's the challenge?
We also do somewhat silly things with netdev lifetime, because we can't
assume netdev gets freed by netdev_free(). Cleaning up the "embedders"
would be beneficial for multiple reasons.
> itself. But we can add a sanity check in netdev_priv() to catch any
> attempts to access the private data of a dummy struct.
>
> Adjust allocation logic to use the new full structure.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 118c40258d07..b476809d0bae 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1815,6 +1815,8 @@ enum netdev_stat_type {
> NETDEV_PCPU_STAT_DSTATS, /* struct pcpu_dstats */
> };
>
> +#define NETDEV_ALIGN 32
Unless someone knows what this is for it should go.
Align priv to cacheline size.
> /**
> * struct net_device - The DEVICE structure.
> *
> @@ -2665,7 +2673,14 @@ void dev_net_set(struct net_device *dev, struct net *net)
> */
> static inline void *netdev_priv(const struct net_device *dev)
> {
> - return (char *)dev + ALIGN(sizeof(struct net_device), NETDEV_ALIGN);
> + struct net_device_priv *priv;
> +
> + /* Dummy struct net_device have no trailing data. */
> + if (WARN_ON_ONCE(dev->reg_state == NETREG_DUMMY))
> + return NULL;
This is a static inline with roughly 11,000 call sites, according to
a quick grep. Aren't WARN_ONCE() in static inlines creating a "once"
object in every compilation unit where they get used?
> + priv = container_of(dev, struct net_device_priv, dev);
> + return (u8 *)priv->data;
> }
next prev parent reply other threads:[~2024-03-01 6:59 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-29 21:30 [PATCH] netdev: Use flexible array for trailing private bytes Kees Cook
2024-02-29 22:15 ` Gustavo A. R. Silva
2024-03-01 6:59 ` Jakub Kicinski [this message]
2024-03-01 8:03 ` Eric Dumazet
2024-03-01 12:58 ` Alexander Lobakin
2024-03-01 13:25 ` Eric Dumazet
2024-03-01 14:30 ` Alexander Lobakin
2024-03-01 17:35 ` Jakub Kicinski
2024-03-04 14:32 ` Alexander Lobakin
2024-03-04 15:24 ` Jakub Kicinski
2024-03-01 11:41 ` Greg KH
2024-03-06 13:16 ` Breno Leitao
2024-03-06 15:06 ` Jakub Kicinski
2024-03-06 23:42 ` Kees Cook
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=20240229225910.79e224cf@kernel.org \
--to=kuba@kernel.org \
--cc=amritha.nambiar@intel.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=gustavoars@kernel.org \
--cc=horms@kernel.org \
--cc=jiri@resnulli.us \
--cc=keescook@chromium.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lixiaoyan@google.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.