From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from omta040.useast.a.cloudfilter.net (omta040.useast.a.cloudfilter.net [44.202.169.39]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 86BCA6D504 for ; Thu, 29 Feb 2024 22:15:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=44.202.169.39 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709244919; cv=none; b=d1YIG8esWl/q9JM7fmL5/NINJ0Z+Dq1UF9Z3LFkW5LqCm4pykw5eEq4rio8QnQNF3GMohTfQj3tmIu4P+4iJl/nXwd6lTcMttFrVMbh/VvbBFECzqj6roQ9U5vTOkzqh/1tOkZO6RMP8BqEovSZFICtAmbyJwuE3ZvbgY2oPy+s= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709244919; c=relaxed/simple; bh=5FSiTuqHmDsjXTZPDXr3lj4oUQySDL+fFiia69OcWH4=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=AzOpUqEF60MMe8IPJeIsjkrr0qscpOs0ErHNnms8tmP2CwcpWSYEzqRrrvmTLkjohygnWaMnALrH1zX9oSkiKyf2DnMV6lHLD0WAvPIzkqhn4qnB+s2TCMA9LAj+9z9upu/aMxFlsyaNW9IXgbIIFOTjXeg/J8faK/BfkCCfPss= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=embeddedor.com; spf=pass smtp.mailfrom=embeddedor.com; dkim=pass (2048-bit key) header.d=embeddedor.com header.i=@embeddedor.com header.b=e0yP1pzt; arc=none smtp.client-ip=44.202.169.39 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=embeddedor.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=embeddedor.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=embeddedor.com header.i=@embeddedor.com header.b="e0yP1pzt" Received: from eig-obgw-6007a.ext.cloudfilter.net ([10.0.30.247]) by cmsmtp with ESMTPS id fnAgrwOpEl9dRfogOrQHCD; Thu, 29 Feb 2024 22:15:16 +0000 Received: from gator4166.hostgator.com ([108.167.133.22]) by cmsmtp with ESMTPS id fogNrVbrbsGJsfogNrMOnk; Thu, 29 Feb 2024 22:15:15 +0000 X-Authority-Analysis: v=2.4 cv=fI4/34ae c=1 sm=1 tr=0 ts=65e101f3 a=1YbLdUo/zbTtOZ3uB5T3HA==:117 a=VhncohosazJxI00KdYJ/5A==:17 a=IkcTkHD0fZMA:10 a=k7vzHIieQBIA:10 a=wYkD_t78qR0A:10 a=cm27Pg_UAAAA:8 a=VwQbUJbxAAAA:8 a=J1Y8HTJGAAAA:8 a=1XWaLZrsAAAA:8 a=20KFwNOVAAAA:8 a=QyXUC8HyAAAA:8 a=xfJypzPxiScX5pcAhuAA:9 a=QEXdDO2ut3YA:10 a=xmb-EsYY8bH0VWELuYED:22 a=AjGcO6oz07-iQ99wixmX:22 a=y1Q9-5lHfBjTkpIzbSAN:22 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=embeddedor.com; s=default; h=Content-Transfer-Encoding:Content-Type: In-Reply-To:From:References:Cc:To:Subject:MIME-Version:Date:Message-ID:Sender :Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help: List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=cf1DZ2erRbMxD8JvVLJPv18i9Hg6aww57G4W3t9Oeoc=; b=e0yP1pztbkxkhpcJxYDiCSOm9p fihMzZKmaTkeqIBnkZZupDuJlp5aNCxO2ey9rO97gUBe4WSG8LzztnW4JPppdUM6t1XjnLPbt1CLP bWp+VnMM7SkP+NJpbTAmj6GdeKiGJU69QxgmuFo4zx4EPNUDjrhSXLipqVR7anVOJdv9OOLDjtbDG xu3x7Ww1m0CEEA1WCr5l0lDJxqPh99MM5R7NpSOKYbs4drDgFiWxObM2b2SL8JobMShCccoSZ/rc+ iVH/f3WVNC4K9jVImTVChD53aiFwo3zDZfX/oLxl2aD/4xQC8HIhRIX9AZ3/jdrBgBDIMelq8rgX2 M8Az3Fzw==; Received: from [201.172.172.225] (port=53434 helo=[192.168.15.10]) by gator4166.hostgator.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.96.2) (envelope-from ) id 1rfogM-002d6M-1s; Thu, 29 Feb 2024 16:15:14 -0600 Message-ID: <93cbd0a3-60cf-400f-a05c-f81dc3d8c75d@embeddedor.com> Date: Thu, 29 Feb 2024 16:15:12 -0600 Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] netdev: Use flexible array for trailing private bytes Content-Language: en-US To: Kees Cook , Jakub Kicinski Cc: "David S. Miller" , Eric Dumazet , Paolo Abeni , Andy Shevchenko , "Gustavo A. R. Silva" , netdev@vger.kernel.org, linux-hardening@vger.kernel.org, Simon Horman , Jiri Pirko , Daniel Borkmann , Coco Li , Amritha Nambiar , linux-kernel@vger.kernel.org References: <20240229213018.work.556-kees@kernel.org> From: "Gustavo A. R. Silva" In-Reply-To: <20240229213018.work.556-kees@kernel.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - gator4166.hostgator.com X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - embeddedor.com X-BWhitelist: no X-Source-IP: 201.172.172.225 X-Source-L: No X-Exim-ID: 1rfogM-002d6M-1s X-Source: X-Source-Args: X-Source-Dir: X-Source-Sender: ([192.168.15.10]) [201.172.172.225]:53434 X-Source-Auth: gustavo@embeddedor.com X-Email-Count: 8 X-Org: HG=hgshared;ORG=hostgator; X-Source-Cap: Z3V6aWRpbmU7Z3V6aWRpbmU7Z2F0b3I0MTY2Lmhvc3RnYXRvci5jb20= X-Local-Domain: yes X-CMAE-Envelope: MS4xfMK5bS4+AiPrjaX1ori8CYyWs7tcUjiirHHEkyzN9GVx3/UD5sY1uFoDgu0nJmxhOS9zVAQcf17v862cYBEZ1EtrX9SHCqqvfYrsZfUI1mm79z1QFHk3 Aya1dB1PuIAk1iBJokDZ0Z50yJuvuweYjdEHfyeGt78G6PnqVA3WNHflyRZocArEFeeK2AnlfjD6kXM9lnKw8SuX7g9ii3a/1jo= 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 Reviewed-by: Gustavo A. R. Silva [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 > Cc: "David S. Miller" > Cc: Eric Dumazet > Cc: Paolo Abeni > Cc: Andy Shevchenko > Cc: "Gustavo A. R. Silva" > Cc: netdev@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);