Netdev List
 help / color / mirror / Atom feed
* [PATCH net] packet: use a consistent hard_header_len in send paths
@ 2026-07-21  8:49 Qihang
  2026-07-21 16:01 ` Daniel Zahka
  2026-07-26  9:21 ` [PATCH net v2] packet: use consistent header lengths in raw " Qihang
  0 siblings, 2 replies; 7+ messages in thread
From: Qihang @ 2026-07-21  8:49 UTC (permalink / raw)
  To: netdev
  Cc: willemdebruijn.kernel, davem, edumazet, kuba, pabeni, horms,
	Qihang, stable

packet_snd() and tpacket_snd() read dev->hard_header_len multiple times
while building an skb. Device reconfiguration can change this value
concurrently, for example through bonding device type changes.

For SOCK_RAW, packet_snd() stores the first value in reserve, later
allocates headroom using LL_RESERVED_SPACE(dev), and then subtracts
reserve from the skb headroom. If hard_header_len decreases between the
reads, the skb can be allocated with less headroom than reserve, moving
skb->data before skb->head. The subsequent skb_copy_datagram_from_iter()
can then attempt an out-of-bounds copy. Hardened usercopy catches this as
a kernel memory overwrite attempt.

tpacket_snd() has the same issue because its allocation and
tpacket_fill_skb() use separate hard_header_len reads.

Read hard_header_len once in each send path and use that snapshot
consistently for skb allocation and construction.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Qihang <q.h.hack.winter@gmail.com>
---
 net/packet/af_packet.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 8e6f3a734ba0..d29e59273054 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2569,6 +2569,7 @@ static int packet_snd_vnet_parse(struct msghdr *msg, size_t *len,
 static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
 		void *frame, struct net_device *dev, void *data, int tp_len,
 		__be16 proto, unsigned char *addr, int hlen, int copylen,
+		int hard_header_len,
 		const struct sockcm_cookie *sockc)
 {
 	union tpacket_uhdr ph;
@@ -2600,8 +2601,8 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
 	} else if (copylen) {
 		int hdrlen = min_t(int, copylen, tp_len);
 
-		skb_push(skb, dev->hard_header_len);
-		skb_put(skb, copylen - dev->hard_header_len);
+		skb_push(skb, hard_header_len);
+		skb_put(skb, copylen - hard_header_len);
 		err = skb_store_bits(skb, 0, data, hdrlen);
 		if (unlikely(err))
 			return err;
@@ -2732,7 +2733,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
 	void *data;
 	int len_sum = 0;
 	int status = TP_STATUS_AVAILABLE;
-	int hlen, tlen, copylen = 0;
+	int hard_header_len, hlen, tlen, copylen = 0;
 	long timeo;
 
 	mutex_lock(&po->pg_vec_lock);
@@ -2779,8 +2780,9 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
 			goto out_put;
 	}
 
+	hard_header_len = READ_ONCE(dev->hard_header_len);
 	if (po->sk.sk_socket->type == SOCK_RAW)
-		reserve = dev->hard_header_len;
+		reserve = hard_header_len;
 	size_max = po->tx_ring.frame_size
 		- (po->tp_hdrlen - sizeof(struct sockaddr_ll));
 
@@ -2817,7 +2819,8 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
 			goto tpacket_error;
 
 		status = TP_STATUS_SEND_REQUEST;
-		hlen = LL_RESERVED_SPACE(dev);
+		hlen = ((hard_header_len + READ_ONCE(dev->needed_headroom)) &
+			~(HH_DATA_MOD - 1)) + HH_DATA_MOD;
 		tlen = dev->needed_tailroom;
 		if (vnet_hdr_sz) {
 			data += vnet_hdr_sz;
@@ -2835,10 +2838,10 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
 						    vnet_hdr.hdr_len);
 			has_vnet_hdr = true;
 		}
-		copylen = max_t(int, copylen, dev->hard_header_len);
+		copylen = max_t(int, copylen, hard_header_len);
 		skb = sock_alloc_send_skb(&po->sk,
 				hlen + tlen + sizeof(struct sockaddr_ll) +
-				(copylen - dev->hard_header_len),
+				(copylen - hard_header_len),
 				!need_wait, &err);
 
 		if (unlikely(skb == NULL)) {
@@ -2848,7 +2851,8 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
 			goto out_status;
 		}
 		tp_len = tpacket_fill_skb(po, skb, ph, dev, data, tp_len, proto,
-					  addr, hlen, copylen, &sockc);
+					  addr, hlen, copylen, hard_header_len,
+					  &sockc);
 		if (likely(tp_len >= 0) &&
 		    tp_len > dev->mtu + reserve &&
 		    !vnet_hdr_sz &&
@@ -2956,7 +2960,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
 	int offset = 0;
 	struct packet_sock *po = pkt_sk(sk);
 	int vnet_hdr_sz = READ_ONCE(po->vnet_hdr_sz);
-	int hlen, tlen, linear;
+	int hard_header_len, hlen, tlen, linear;
 	int extra_len = 0;
 
 	/*
@@ -2996,14 +3000,16 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
 			goto out_unlock;
 	}
 
-	if (sock->type == SOCK_RAW)
-		reserve = dev->hard_header_len;
 	if (vnet_hdr_sz) {
 		err = packet_snd_vnet_parse(msg, &len, &vnet_hdr, vnet_hdr_sz);
 		if (err)
 			goto out_unlock;
 	}
 
+	hard_header_len = READ_ONCE(dev->hard_header_len);
+	if (sock->type == SOCK_RAW)
+		reserve = hard_header_len;
+
 	if (unlikely(sock_flag(sk, SOCK_NOFCS))) {
 		if (!netif_supports_nofcs(dev)) {
 			err = -EPROTONOSUPPORT;
@@ -3018,10 +3024,11 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
 		goto out_unlock;
 
 	err = -ENOBUFS;
-	hlen = LL_RESERVED_SPACE(dev);
+	hlen = ((hard_header_len + READ_ONCE(dev->needed_headroom)) &
+		~(HH_DATA_MOD - 1)) + HH_DATA_MOD;
 	tlen = dev->needed_tailroom;
 	linear = __virtio16_to_cpu(vio_le(), vnet_hdr.hdr_len);
-	linear = max(linear, min_t(int, len, dev->hard_header_len));
+	linear = max(linear, min_t(int, len, hard_header_len));
 	skb = packet_alloc_skb(sk, hlen + tlen, hlen, len, linear,
 			       msg->msg_flags & MSG_DONTWAIT, &err);
 	if (skb == NULL)
@@ -3037,7 +3044,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
 	} else if (reserve) {
 		skb_reserve(skb, -reserve);
 		if (len < reserve + sizeof(struct ipv6hdr) &&
-		    dev->min_header_len != dev->hard_header_len)
+		    dev->min_header_len != hard_header_len)
 			skb_reset_network_header(skb);
 	}
 
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net] packet: use a consistent hard_header_len in send paths
  2026-07-21  8:49 [PATCH net] packet: use a consistent hard_header_len in send paths Qihang
@ 2026-07-21 16:01 ` Daniel Zahka
  2026-07-22 14:26   ` Willem de Bruijn
  2026-07-26  9:21 ` [PATCH net v2] packet: use consistent header lengths in raw " Qihang
  1 sibling, 1 reply; 7+ messages in thread
From: Daniel Zahka @ 2026-07-21 16:01 UTC (permalink / raw)
  To: Qihang, netdev
  Cc: willemdebruijn.kernel, davem, edumazet, kuba, pabeni, horms,
	stable



On 7/21/26 4:49 AM, Qihang wrote:
> packet_snd() and tpacket_snd() read dev->hard_header_len multiple times
> while building an skb. Device reconfiguration can change this value
> concurrently, for example through bonding device type changes.
> 
> For SOCK_RAW, packet_snd() stores the first value in reserve, later
> allocates headroom using LL_RESERVED_SPACE(dev), and then subtracts
> reserve from the skb headroom. If hard_header_len decreases between the
> reads, the skb can be allocated with less headroom than reserve, moving
> skb->data before skb->head. The subsequent skb_copy_datagram_from_iter()
> can then attempt an out-of-bounds copy. Hardened usercopy catches this as
> a kernel memory overwrite attempt.
> 

Wouldn't there be a similar issue in the SOCK_DGRAM path with 
dev_hard_header() calling skb_push() after packet_alloc_skb()?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] packet: use a consistent hard_header_len in send paths
  2026-07-21 16:01 ` Daniel Zahka
@ 2026-07-22 14:26   ` Willem de Bruijn
  2026-07-24  3:36     ` Qihang
  0 siblings, 1 reply; 7+ messages in thread
From: Willem de Bruijn @ 2026-07-22 14:26 UTC (permalink / raw)
  To: Daniel Zahka, Qihang, netdev
  Cc: willemdebruijn.kernel, davem, edumazet, kuba, pabeni, horms,
	stable

Daniel Zahka wrote:
> 
> 
> On 7/21/26 4:49 AM, Qihang wrote:
> > packet_snd() and tpacket_snd() read dev->hard_header_len multiple times
> > while building an skb. Device reconfiguration can change this value
> > concurrently, for example through bonding device type changes.
> > 
> > For SOCK_RAW, packet_snd() stores the first value in reserve, later
> > allocates headroom using LL_RESERVED_SPACE(dev), and then subtracts
> > reserve from the skb headroom. If hard_header_len decreases between the
> > reads, the skb can be allocated with less headroom than reserve, moving
> > skb->data before skb->head. The subsequent skb_copy_datagram_from_iter()
> > can then attempt an out-of-bounds copy. Hardened usercopy catches this as
> > a kernel memory overwrite attempt.
> > 
> 
> Wouldn't there be a similar issue in the SOCK_DGRAM path with 
> dev_hard_header() calling skb_push() after packet_alloc_skb()?

Good point. this does not use hard_header_len directly, but e.g.,
eth_header() assumes ETH_HLEN is available to push.

Simply caching hard_header_len won't resolve this. dev->hard_header_len
and dev->header_ops (incl .create) are not updated atomically.

I missed this earlier, but besides packet_snd and tpacket_snd, the fix
is also needed by packet_sendmsg_spkt.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] packet: use a consistent hard_header_len in send paths
  2026-07-22 14:26   ` Willem de Bruijn
@ 2026-07-24  3:36     ` Qihang
  2026-07-25  7:39       ` Willem de Bruijn
  0 siblings, 1 reply; 7+ messages in thread
From: Qihang @ 2026-07-24  3:36 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Daniel Zahka, netdev, davem, edumazet, kuba, pabeni, horms,
	stable

Agreed. Caching hard_header_len fixes the RAW paths' inconsistent skb
arithmetic, but not the SOCK_DGRAM race between headroom allocation and
header_ops->create().

I also missed packet_sendmsg_spkt(), which needs the same RAW-path fix.

Would you prefer a v2 covering the three RAW paths, with the DGRAM
callback headroom issue handled separately, or both issues in one
series?

On Wed, Jul 22, 2026 at 10:26 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> Daniel Zahka wrote:
> >
> >
> > On 7/21/26 4:49 AM, Qihang wrote:
> > > packet_snd() and tpacket_snd() read dev->hard_header_len multiple times
> > > while building an skb. Device reconfiguration can change this value
> > > concurrently, for example through bonding device type changes.
> > >
> > > For SOCK_RAW, packet_snd() stores the first value in reserve, later
> > > allocates headroom using LL_RESERVED_SPACE(dev), and then subtracts
> > > reserve from the skb headroom. If hard_header_len decreases between the
> > > reads, the skb can be allocated with less headroom than reserve, moving
> > > skb->data before skb->head. The subsequent skb_copy_datagram_from_iter()
> > > can then attempt an out-of-bounds copy. Hardened usercopy catches this as
> > > a kernel memory overwrite attempt.
> > >
> >
> > Wouldn't there be a similar issue in the SOCK_DGRAM path with
> > dev_hard_header() calling skb_push() after packet_alloc_skb()?
>
> Good point. this does not use hard_header_len directly, but e.g.,
> eth_header() assumes ETH_HLEN is available to push.
>
> Simply caching hard_header_len won't resolve this. dev->hard_header_len
> and dev->header_ops (incl .create) are not updated atomically.
>
> I missed this earlier, but besides packet_snd and tpacket_snd, the fix
> is also needed by packet_sendmsg_spkt.
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] packet: use a consistent hard_header_len in send paths
  2026-07-24  3:36     ` Qihang
@ 2026-07-25  7:39       ` Willem de Bruijn
  0 siblings, 0 replies; 7+ messages in thread
From: Willem de Bruijn @ 2026-07-25  7:39 UTC (permalink / raw)
  To: Qihang, Willem de Bruijn
  Cc: Daniel Zahka, netdev, davem, edumazet, kuba, pabeni, horms,
	stable

Qihang wrote:
> Agreed. Caching hard_header_len fixes the RAW paths' inconsistent skb
> arithmetic, but not the SOCK_DGRAM race between headroom allocation and
> header_ops->create().
> 
> I also missed packet_sendmsg_spkt(), which needs the same RAW-path fix.
> 
> Would you prefer a v2 covering the three RAW paths, with the DGRAM
> callback headroom issue handled separately, or both issues in one
> series?

Since the DGRAM path is non-trivial, better to fix that in a separate
patch. Can be same series, but no need to delay the RAW patch for
that. Thanks.
 
> On Wed, Jul 22, 2026 at 10:26 PM Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
> >
> > Daniel Zahka wrote:
> > >
> > >
> > > On 7/21/26 4:49 AM, Qihang wrote:
> > > > packet_snd() and tpacket_snd() read dev->hard_header_len multiple times
> > > > while building an skb. Device reconfiguration can change this value
> > > > concurrently, for example through bonding device type changes.
> > > >
> > > > For SOCK_RAW, packet_snd() stores the first value in reserve, later
> > > > allocates headroom using LL_RESERVED_SPACE(dev), and then subtracts
> > > > reserve from the skb headroom. If hard_header_len decreases between the
> > > > reads, the skb can be allocated with less headroom than reserve, moving
> > > > skb->data before skb->head. The subsequent skb_copy_datagram_from_iter()
> > > > can then attempt an out-of-bounds copy. Hardened usercopy catches this as
> > > > a kernel memory overwrite attempt.
> > > >
> > >
> > > Wouldn't there be a similar issue in the SOCK_DGRAM path with
> > > dev_hard_header() calling skb_push() after packet_alloc_skb()?
> >
> > Good point. this does not use hard_header_len directly, but e.g.,
> > eth_header() assumes ETH_HLEN is available to push.
> >
> > Simply caching hard_header_len won't resolve this. dev->hard_header_len
> > and dev->header_ops (incl .create) are not updated atomically.
> >
> > I missed this earlier, but besides packet_snd and tpacket_snd, the fix
> > is also needed by packet_sendmsg_spkt.
> >



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net v2] packet: use consistent header lengths in raw send paths
  2026-07-21  8:49 [PATCH net] packet: use a consistent hard_header_len in send paths Qihang
  2026-07-21 16:01 ` Daniel Zahka
@ 2026-07-26  9:21 ` Qihang
  2026-07-26 15:21   ` Willem de Bruijn
  1 sibling, 1 reply; 7+ messages in thread
From: Qihang @ 2026-07-26  9:21 UTC (permalink / raw)
  To: netdev
  Cc: willemdebruijn.kernel, daniel.zahka, davem, edumazet, kuba,
	pabeni, horms, stable, Qihang

packet_snd(), tpacket_snd(), and packet_sendmsg_spkt() read
dev->hard_header_len multiple times while allocating and constructing an
skb. Device reconfiguration can change this value concurrently, for
example through bonding device type changes.

For SOCK_RAW, packet_snd() can save a larger value in reserve and later
allocate headroom using a smaller value. Moving skb->data back by reserve
then places it before skb->head, and the following copy from userspace can
attempt an out-of-bounds write.

tpacket_snd() similarly uses independent values for allocation and for
the skb_push()/skb_put() operations in tpacket_fill_skb(). The legacy
packet_sendmsg_spkt() path also calculates its reservation and header
offset from separate reads before dropping the RCU read lock to allocate
the skb. A larger offset than reservation can move skb->data before
skb->head, while mixed TX-ring values can also make
copylen - hard_header_len negative.

Read hard_header_len once for each send operation and use that value for
RAW allocation and construction. The trigger requires racing an
AF_PACKET sender with privileged netdevice reconfiguration.

The separate SOCK_DGRAM consistency problem between hard_header_len and
header_ops->create is not addressed here.

Fixes: b84bbaf7a6c8 ("packet: in packet_snd start writing at link layer allocation")
Fixes: 69e3c75f4d54 ("net: TX_RING and packet mmap")
Cc: stable@vger.kernel.org
Signed-off-by: Qihang <q.h.hack.winter@gmail.com>
---
v2:
- Cover packet_sendmsg_spkt().
- Limit snapshot-based calculations to RAW construction; leave DGRAM
  unchanged for a separate fix.

Link to v1: https://lore.kernel.org/netdev/20260721084935.12312-1-q.h.hack.winter@gmail.com/
---
 net/packet/af_packet.c | 64 ++++++++++++++++++++++++++++++------------
 1 file changed, 46 insertions(+), 18 deletions(-)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index e75d2932475a..d23898f24641 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1953,6 +1953,7 @@ static int packet_sendmsg_spkt(struct socket *sock, struct msghdr *msg,
 	struct net_device *dev;
 	struct sockcm_cookie sockc;
 	__be16 proto = 0;
+	unsigned int hard_header_len = 0;
 	int err;
 	int extra_len = 0;
 
@@ -1996,15 +1997,21 @@ static int packet_sendmsg_spkt(struct socket *sock, struct msghdr *msg,
 		}
 		extra_len = 4; /* We're doing our own CRC */
 	}
+	if (!skb)
+		hard_header_len = READ_ONCE(dev->hard_header_len);
 
 	err = -EMSGSIZE;
-	if (len > dev->mtu + dev->hard_header_len + VLAN_HLEN + extra_len)
+	if (len > dev->mtu + hard_header_len + VLAN_HLEN + extra_len)
 		goto out_unlock;
 
 	if (!skb) {
-		size_t reserved = LL_RESERVED_SPACE(dev);
+		size_t reserved;
 		int tlen = dev->needed_tailroom;
-		unsigned int hhlen = dev->header_ops ? dev->hard_header_len : 0;
+		unsigned int hhlen = READ_ONCE(dev->header_ops) ?
+				     hard_header_len : 0;
+
+		reserved = ((hard_header_len + READ_ONCE(dev->needed_headroom)) &
+			    ~(HH_DATA_MOD - 1)) + HH_DATA_MOD;
 
 		rcu_read_unlock();
 		skb = sock_wmalloc(sk, len + reserved + tlen, 0, GFP_KERNEL);
@@ -2569,6 +2576,7 @@ static int packet_snd_vnet_parse(struct msghdr *msg, size_t *len,
 static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
 		void *frame, struct net_device *dev, void *data, int tp_len,
 		__be16 proto, unsigned char *addr, int hlen, int copylen,
+		int hard_header_len,
 		const struct sockcm_cookie *sockc)
 {
 	union tpacket_uhdr ph;
@@ -2600,8 +2608,8 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
 	} else if (copylen) {
 		int hdrlen = min_t(int, copylen, tp_len);
 
-		skb_push(skb, dev->hard_header_len);
-		skb_put(skb, copylen - dev->hard_header_len);
+		skb_push(skb, hard_header_len);
+		skb_put(skb, copylen - hard_header_len);
 		err = skb_store_bits(skb, 0, data, hdrlen);
 		if (unlikely(err))
 			return err;
@@ -2732,7 +2740,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
 	void *data;
 	int len_sum = 0;
 	int status = TP_STATUS_AVAILABLE;
-	int hlen, tlen, copylen = 0;
+	int hard_header_len = 0, hlen, tlen, copylen = 0;
 	long timeo;
 
 	mutex_lock(&po->pg_vec_lock);
@@ -2779,8 +2787,10 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
 			goto out_put;
 	}
 
-	if (po->sk.sk_socket->type == SOCK_RAW)
-		reserve = dev->hard_header_len;
+	if (po->sk.sk_socket->type == SOCK_RAW) {
+		hard_header_len = READ_ONCE(dev->hard_header_len);
+		reserve = hard_header_len;
+	}
 	size_max = po->tx_ring.frame_size
 		- (po->tp_hdrlen - sizeof(struct sockaddr_ll));
 
@@ -2817,7 +2827,12 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
 			goto tpacket_error;
 
 		status = TP_STATUS_SEND_REQUEST;
-		hlen = LL_RESERVED_SPACE(dev);
+		if (po->sk.sk_socket->type == SOCK_RAW)
+			hlen = ((hard_header_len +
+				 READ_ONCE(dev->needed_headroom)) &
+				~(HH_DATA_MOD - 1)) + HH_DATA_MOD;
+		else
+			hlen = LL_RESERVED_SPACE(dev);
 		tlen = dev->needed_tailroom;
 		if (vnet_hdr_sz) {
 			data += vnet_hdr_sz;
@@ -2835,10 +2850,14 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
 						    vnet_hdr.hdr_len);
 			has_vnet_hdr = true;
 		}
-		copylen = max_t(int, copylen, dev->hard_header_len);
+		if (po->sk.sk_socket->type == SOCK_RAW)
+			copylen = max_t(int, copylen, hard_header_len);
+		else
+			copylen = max_t(int, copylen, dev->hard_header_len);
 		skb = sock_alloc_send_skb(&po->sk,
 				hlen + tlen + sizeof(struct sockaddr_ll) +
-				(copylen - dev->hard_header_len),
+				(copylen - (po->sk.sk_socket->type == SOCK_RAW ?
+				 hard_header_len : dev->hard_header_len)),
 				!need_wait, &err);
 
 		if (unlikely(skb == NULL)) {
@@ -2848,7 +2867,8 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
 			goto out_status;
 		}
 		tp_len = tpacket_fill_skb(po, skb, ph, dev, data, tp_len, proto,
-					  addr, hlen, copylen, &sockc);
+					  addr, hlen, copylen, hard_header_len,
+					  &sockc);
 		if (likely(tp_len >= 0) &&
 		    tp_len > dev->mtu + reserve &&
 		    !vnet_hdr_sz &&
@@ -2956,7 +2976,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
 	int offset = 0;
 	struct packet_sock *po = pkt_sk(sk);
 	int vnet_hdr_sz = READ_ONCE(po->vnet_hdr_sz);
-	int hlen, tlen, linear;
+	int hard_header_len = 0, hlen, tlen, linear;
 	int extra_len = 0;
 
 	/*
@@ -2996,14 +3016,17 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
 			goto out_unlock;
 	}
 
-	if (sock->type == SOCK_RAW)
-		reserve = dev->hard_header_len;
 	if (vnet_hdr_sz) {
 		err = packet_snd_vnet_parse(msg, &len, &vnet_hdr, vnet_hdr_sz);
 		if (err)
 			goto out_unlock;
 	}
 
+	if (sock->type == SOCK_RAW) {
+		hard_header_len = READ_ONCE(dev->hard_header_len);
+		reserve = hard_header_len;
+	}
+
 	if (unlikely(sock_flag(sk, SOCK_NOFCS))) {
 		if (!netif_supports_nofcs(dev)) {
 			err = -EPROTONOSUPPORT;
@@ -3018,10 +3041,15 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
 		goto out_unlock;
 
 	err = -ENOBUFS;
-	hlen = LL_RESERVED_SPACE(dev);
+	if (sock->type == SOCK_RAW)
+		hlen = ((hard_header_len + READ_ONCE(dev->needed_headroom)) &
+			~(HH_DATA_MOD - 1)) + HH_DATA_MOD;
+	else
+		hlen = LL_RESERVED_SPACE(dev);
 	tlen = dev->needed_tailroom;
 	linear = __virtio16_to_cpu(vio_le(), vnet_hdr.hdr_len);
-	linear = max(linear, min_t(int, len, dev->hard_header_len));
+	linear = max(linear, min_t(int, len, sock->type == SOCK_RAW ?
+					 hard_header_len : dev->hard_header_len));
 	skb = packet_alloc_skb(sk, hlen + tlen, hlen, len, linear,
 			       msg->msg_flags & MSG_DONTWAIT, &err);
 	if (skb == NULL)
@@ -3037,7 +3065,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
 	} else if (reserve) {
 		skb_reserve(skb, -reserve);
 		if (len < reserve + sizeof(struct ipv6hdr) &&
-		    dev->min_header_len != dev->hard_header_len)
+		    dev->min_header_len != hard_header_len)
 			skb_reset_network_header(skb);
 	}
 
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net v2] packet: use consistent header lengths in raw send paths
  2026-07-26  9:21 ` [PATCH net v2] packet: use consistent header lengths in raw " Qihang
@ 2026-07-26 15:21   ` Willem de Bruijn
  0 siblings, 0 replies; 7+ messages in thread
From: Willem de Bruijn @ 2026-07-26 15:21 UTC (permalink / raw)
  To: Qihang, netdev
  Cc: willemdebruijn.kernel, daniel.zahka, davem, edumazet, kuba,
	pabeni, horms, stable, Qihang

Qihang wrote:
> packet_snd(), tpacket_snd(), and packet_sendmsg_spkt() read
> dev->hard_header_len multiple times while allocating and constructing an
> skb. Device reconfiguration can change this value concurrently, for
> example through bonding device type changes.
> 
> For SOCK_RAW, packet_snd() can save a larger value in reserve and later
> allocate headroom using a smaller value. Moving skb->data back by reserve
> then places it before skb->head, and the following copy from userspace can
> attempt an out-of-bounds write.
> 
> tpacket_snd() similarly uses independent values for allocation and for
> the skb_push()/skb_put() operations in tpacket_fill_skb(). The legacy
> packet_sendmsg_spkt() path also calculates its reservation and header
> offset from separate reads before dropping the RCU read lock to allocate
> the skb. A larger offset than reservation can move skb->data before
> skb->head, while mixed TX-ring values can also make
> copylen - hard_header_len negative.
> 
> Read hard_header_len once for each send operation and use that value for
> RAW allocation and construction. The trigger requires racing an
> AF_PACKET sender with privileged netdevice reconfiguration.
> 
> The separate SOCK_DGRAM consistency problem between hard_header_len and
> header_ops->create is not addressed here.
> 
> Fixes: b84bbaf7a6c8 ("packet: in packet_snd start writing at link layer allocation")
> Fixes: 69e3c75f4d54 ("net: TX_RING and packet mmap")

Not sure whether a patch can have two fixes tags.
Maybe better two separate patches.

> Cc: stable@vger.kernel.org
> Signed-off-by: Qihang <q.h.hack.winter@gmail.com>
> ---
> v2:
> - Cover packet_sendmsg_spkt().
> - Limit snapshot-based calculations to RAW construction; leave DGRAM
>   unchanged for a separate fix.
> 
> Link to v1: https://lore.kernel.org/netdev/20260721084935.12312-1-q.h.hack.winter@gmail.com/
> ---
>  net/packet/af_packet.c | 64 ++++++++++++++++++++++++++++++------------
>  1 file changed, 46 insertions(+), 18 deletions(-)
> 
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index e75d2932475a..d23898f24641 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1953,6 +1953,7 @@ static int packet_sendmsg_spkt(struct socket *sock, struct msghdr *msg,
>  	struct net_device *dev;
>  	struct sockcm_cookie sockc;
>  	__be16 proto = 0;
> +	unsigned int hard_header_len = 0;

Please keep reverse xmas tree ordering when not too inconvenient.

>  	int err;
>  	int extra_len = 0;
>  
> @@ -1996,15 +1997,21 @@ static int packet_sendmsg_spkt(struct socket *sock, struct msghdr *msg,
>  		}
>  		extra_len = 4; /* We're doing our own CRC */
>  	}
> +	if (!skb)

Why this branch?

> +		hard_header_len = READ_ONCE(dev->hard_header_len);
>
>  	err = -EMSGSIZE;
> -	if (len > dev->mtu + dev->hard_header_len + VLAN_HLEN + extra_len)
> +	if (len > dev->mtu + hard_header_len + VLAN_HLEN + extra_len)
>  		goto out_unlock;
>  
>  	if (!skb) {
> -		size_t reserved = LL_RESERVED_SPACE(dev);
> +		size_t reserved;
>  		int tlen = dev->needed_tailroom;
> -		unsigned int hhlen = dev->header_ops ? dev->hard_header_len : 0;
> +		unsigned int hhlen = READ_ONCE(dev->header_ops) ?

Why add this READ_ONCE? Other header_ops readers lack this.

> +				     hard_header_len : 0;
> +
> +		reserved = ((hard_header_len + READ_ONCE(dev->needed_headroom)) &
> +			    ~(HH_DATA_MOD - 1)) + HH_DATA_MOD;

Now that this file uses this three times, it might be worthwhile
creating an LL_RESERVED_SPACE_EX(dev, hhlen) wrapper. And have
LL_RESERVED_SPACE call that. (optional).
>  
>  		rcu_read_unlock();
>  		skb = sock_wmalloc(sk, len + reserved + tlen, 0, GFP_KERNEL);
> @@ -2569,6 +2576,7 @@ static int packet_snd_vnet_parse(struct msghdr *msg, size_t *len,
>  static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
>  		void *frame, struct net_device *dev, void *data, int tp_len,
>  		__be16 proto, unsigned char *addr, int hlen, int copylen,
> +		int hard_header_len,

Inconsistent unsigned int above vs int here. Dev field is unsigned
short. Int is fine.

>  		const struct sockcm_cookie *sockc)
>  {
>  	union tpacket_uhdr ph;
> @@ -2600,8 +2608,8 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
>  	} else if (copylen) {
>  		int hdrlen = min_t(int, copylen, tp_len);
>  
> -		skb_push(skb, dev->hard_header_len);
> -		skb_put(skb, copylen - dev->hard_header_len);
> +		skb_push(skb, hard_header_len);
> +		skb_put(skb, copylen - hard_header_len);
>  		err = skb_store_bits(skb, 0, data, hdrlen);
>  		if (unlikely(err))
>  			return err;
> @@ -2732,7 +2740,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
>  	void *data;
>  	int len_sum = 0;
>  	int status = TP_STATUS_AVAILABLE;
> -	int hlen, tlen, copylen = 0;
> +	int hard_header_len = 0, hlen, tlen, copylen = 0;
>  	long timeo;
>  
>  	mutex_lock(&po->pg_vec_lock);
> @@ -2779,8 +2787,10 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
>  			goto out_put;
>  	}
>  
> -	if (po->sk.sk_socket->type == SOCK_RAW)
> -		reserve = dev->hard_header_len;
> +	if (po->sk.sk_socket->type == SOCK_RAW) {
> +		hard_header_len = READ_ONCE(dev->hard_header_len);
> +		reserve = hard_header_len;
> +	}
>  	size_max = po->tx_ring.frame_size
>  		- (po->tp_hdrlen - sizeof(struct sockaddr_ll));
>  
> @@ -2817,7 +2827,12 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
>  			goto tpacket_error;
>  
>  		status = TP_STATUS_SEND_REQUEST;
> -		hlen = LL_RESERVED_SPACE(dev);
> +		if (po->sk.sk_socket->type == SOCK_RAW)
> +			hlen = ((hard_header_len +

Here and elsewhere: why special case this only for SOCK_RAW?
This complicates control flow.

> +				 READ_ONCE(dev->needed_headroom)) &
> +				~(HH_DATA_MOD - 1)) + HH_DATA_MOD;
> +		else
> +			hlen = LL_RESERVED_SPACE(dev);
>  		tlen = dev->needed_tailroom;
>  		if (vnet_hdr_sz) {
>  			data += vnet_hdr_sz;
> @@ -2835,10 +2850,14 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
>  						    vnet_hdr.hdr_len);
>  			has_vnet_hdr = true;
>  		}
> -		copylen = max_t(int, copylen, dev->hard_header_len);
> +		if (po->sk.sk_socket->type == SOCK_RAW)
> +			copylen = max_t(int, copylen, hard_header_len);
> +		else
> +			copylen = max_t(int, copylen, dev->hard_header_len);
>  		skb = sock_alloc_send_skb(&po->sk,
>  				hlen + tlen + sizeof(struct sockaddr_ll) +
> -				(copylen - dev->hard_header_len),
> +				(copylen - (po->sk.sk_socket->type == SOCK_RAW ?
> +				 hard_header_len : dev->hard_header_len)),
>  				!need_wait, &err);
>  
>  		if (unlikely(skb == NULL)) {
> @@ -2848,7 +2867,8 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
>  			goto out_status;
>  		}
>  		tp_len = tpacket_fill_skb(po, skb, ph, dev, data, tp_len, proto,
> -					  addr, hlen, copylen, &sockc);
> +					  addr, hlen, copylen, hard_header_len,
> +					  &sockc);
>  		if (likely(tp_len >= 0) &&
>  		    tp_len > dev->mtu + reserve &&
>  		    !vnet_hdr_sz &&
> @@ -2956,7 +2976,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
>  	int offset = 0;
>  	struct packet_sock *po = pkt_sk(sk);
>  	int vnet_hdr_sz = READ_ONCE(po->vnet_hdr_sz);
> -	int hlen, tlen, linear;
> +	int hard_header_len = 0, hlen, tlen, linear;
>  	int extra_len = 0;
>  
>  	/*
> @@ -2996,14 +3016,17 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
>  			goto out_unlock;
>  	}
>  
> -	if (sock->type == SOCK_RAW)
> -		reserve = dev->hard_header_len;
>  	if (vnet_hdr_sz) {
>  		err = packet_snd_vnet_parse(msg, &len, &vnet_hdr, vnet_hdr_sz);
>  		if (err)
>  			goto out_unlock;
>  	}
>  
> +	if (sock->type == SOCK_RAW) {
> +		hard_header_len = READ_ONCE(dev->hard_header_len);
> +		reserve = hard_header_len;
> +	}
> +
>  	if (unlikely(sock_flag(sk, SOCK_NOFCS))) {
>  		if (!netif_supports_nofcs(dev)) {
>  			err = -EPROTONOSUPPORT;
> @@ -3018,10 +3041,15 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
>  		goto out_unlock;
>  
>  	err = -ENOBUFS;
> -	hlen = LL_RESERVED_SPACE(dev);
> +	if (sock->type == SOCK_RAW)
> +		hlen = ((hard_header_len + READ_ONCE(dev->needed_headroom)) &
> +			~(HH_DATA_MOD - 1)) + HH_DATA_MOD;
> +	else
> +		hlen = LL_RESERVED_SPACE(dev);
>  	tlen = dev->needed_tailroom;
>  	linear = __virtio16_to_cpu(vio_le(), vnet_hdr.hdr_len);
> -	linear = max(linear, min_t(int, len, dev->hard_header_len));
> +	linear = max(linear, min_t(int, len, sock->type == SOCK_RAW ?
> +					 hard_header_len : dev->hard_header_len));
>  	skb = packet_alloc_skb(sk, hlen + tlen, hlen, len, linear,
>  			       msg->msg_flags & MSG_DONTWAIT, &err);
>  	if (skb == NULL)
> @@ -3037,7 +3065,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
>  	} else if (reserve) {
>  		skb_reserve(skb, -reserve);
>  		if (len < reserve + sizeof(struct ipv6hdr) &&
> -		    dev->min_header_len != dev->hard_header_len)
> +		    dev->min_header_len != hard_header_len)
>  			skb_reset_network_header(skb);
>  	}
>  
> -- 
> 2.50.1 (Apple Git-155)
> 



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-07-26 15:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-21  8:49 [PATCH net] packet: use a consistent hard_header_len in send paths Qihang
2026-07-21 16:01 ` Daniel Zahka
2026-07-22 14:26   ` Willem de Bruijn
2026-07-24  3:36     ` Qihang
2026-07-25  7:39       ` Willem de Bruijn
2026-07-26  9:21 ` [PATCH net v2] packet: use consistent header lengths in raw " Qihang
2026-07-26 15:21   ` Willem de Bruijn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox