Re: [PATCH] netdev: Use flexible array for trailing private bytes

[Gustavo A. R. Silva]

On 2/29/24 15:30, 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. As many dummy struct net_device instances exist still,
it is non-trivial to but this flexible array inside struct net_device
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 <keesc...@chromium.org>

Reviewed-by: Gustavo A. R. Silva <gustavo...@kernel.org>
[for the flex `struct net_device_priv`, `struct_size()`, `__counted_by()`,
and the use of `container_of()` to retrieve a pointer to the flex struct
and return pointer to flex-array member `data` in `netdev_priv()`]

Thanks
--
Gustavo

---
Cc: Jakub Kicinski <k...@kernel.org>
Cc: "David S. Miller" <da...@davemloft.net>
Cc: Eric Dumazet <eduma...@google.com>
Cc: Paolo Abeni <pab...@redhat.com>
Cc: Andy Shevchenko <andriy.shevche...@linux.intel.com>
Cc: "Gustavo A. R. Silva" <gustavo...@kernel.org>
Cc: net...@vger.kernel.org
Cc: linux-hardening@vger.kernel.org
---
  include/linux/netdevice.h | 21 ++++++++++++++++++---
  net/core/dev.c            | 12 ++++--------
  2 files changed, 22 insertions(+), 11 deletions(-)

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

+
  /**
   *    struct net_device - The DEVICE structure.
   *
@@ -2476,6 +2478,14 @@ struct net_device {
        struct hlist_head       page_pools;
  #endif
  };
+
+struct net_device_priv {
+       struct net_device       dev;
+       u32                     size;
+       u8                      data[] __counted_by(size)
+                                      __aligned(NETDEV_ALIGN);
+};
+
  #define to_net_dev(d) container_of(d, struct net_device, dev)

/*

@@ -2496,8 +2506,6 @@ static inline bool netif_elide_gro(const struct 
net_device *dev)
        return false;
  }

-#define NETDEV_ALIGN 32

-
  static inline
  int netdev_get_prio_tc_map(const struct net_device *dev, u32 prio)
  {
@@ -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;
+
+       priv = container_of(dev, struct net_device_priv, dev);
+       return (u8 *)priv->data;
  }

/* Set the sysfs physical device reference for the network logical device

diff --git a/net/core/dev.c b/net/core/dev.c
index cb2dab0feee0..0fcaf6ae8486 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10800,7 +10800,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, 
const char *name,
  {
        struct net_device *dev;
        unsigned int alloc_size;
-       struct net_device *p;
+       struct net_device_priv *p;

BUG_ON(strlen(name) >= sizeof(dev->name)); @@ -10814,20 +10814,16 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,

                return NULL;
        }

- alloc_size = sizeof(struct net_device);

-       if (sizeof_priv) {
-               /* ensure 32-byte alignment of private area */
-               alloc_size = ALIGN(alloc_size, NETDEV_ALIGN);
-               alloc_size += sizeof_priv;
-       }
+       alloc_size = struct_size(p, data, sizeof_priv);
        /* ensure 32-byte alignment of whole construct */
        alloc_size += NETDEV_ALIGN - 1;

p = kvzalloc(alloc_size, GFP_KERNEL_ACCOUNT | __GFP_RETRY_MAYFAIL);

        if (!p)
                return NULL;
+       p->size = sizeof_priv;

- dev = PTR_ALIGN(p, NETDEV_ALIGN);

+       dev = &PTR_ALIGN(p, NETDEV_ALIGN)->dev;
        dev->padded = (char *)dev - (char *)p;

ref_tracker_dir_init(&dev->refcnt_tracker, 128, name);