[PATCH net] vlan: ensure sufficient headroom in vlan_dev_hard_header()

[PATCH net] vlan: ensure sufficient headroom in vlan_dev_hard_header()

[Eric Dumazet]

Callers that only reserve ETH_HLEN or less (such as llc_alloc_frame() or
br_send_bpdu()), or skbs allocated before dynamic device/headroom
changes (e.g. toggling VLAN_FLAG_REORDER_HDR or bonding/team switching
slaves), can reach vlan_dev_hard_header() with insufficient headroom and
trigger skb_under_panic().

Use skb_cow_head() in vlan_dev_hard_header() when VLAN_FLAG_REORDER_HDR
is not set to ensure sufficient headroom for the VLAN header(s) and the
underlying device hard header.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Zixuan Chai <petalzu987@gmail.com>
Closes: https://lore.kernel.org/netdev/cover.1789987105.git.petalzu987@gmail.com/
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/8021q/vlan_dev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 2859cbac3f266b7c4e3f44f41280d33ab69c5270..3598ad0779f050d25c6110de476fe9613137f983 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -55,6 +55,9 @@ static int vlan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
 	int rc;
 
 	if (!(vlan->flags & VLAN_FLAG_REORDER_HDR)) {
+		if (skb_cow_head(skb,
+				 dev->hard_header_len + dev->needed_headroom) < 0)
+			return -ENOMEM;
 		vhdr = skb_push(skb, VLAN_HLEN);
 
 		vlan_tci = vlan->vlan_id;
-- 
2.55.0.1082.g2b9226bbc0-goog

Re: [PATCH net] vlan: ensure sufficient headroom in vlan_dev_hard_header()

[Hangbin Liu]

Hi Eric,
On Tue, Sep 22, 2026 at 11:39:24AM +0000, Eric Dumazet wrote:
> Callers that only reserve ETH_HLEN or less (such as llc_alloc_frame() or
> br_send_bpdu()), or skbs allocated before dynamic device/headroom
> changes (e.g. toggling VLAN_FLAG_REORDER_HDR or bonding/team switching
> slaves), can reach vlan_dev_hard_header() with insufficient headroom and
> trigger skb_under_panic().
> 
> Use skb_cow_head() in vlan_dev_hard_header() when VLAN_FLAG_REORDER_HDR
> is not set to ensure sufficient headroom for the VLAN header(s) and the
> underlying device hard header.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: Zixuan Chai <petalzu987@gmail.com>
> Closes: https://lore.kernel.org/netdev/cover.1789987105.git.petalzu987@gmail.com/
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>  net/8021q/vlan_dev.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index 2859cbac3f266b7c4e3f44f41280d33ab69c5270..3598ad0779f050d25c6110de476fe9613137f983 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -55,6 +55,9 @@ static int vlan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
>  	int rc;
>  
>  	if (!(vlan->flags & VLAN_FLAG_REORDER_HDR)) {
> +		if (skb_cow_head(skb,
> +				 dev->hard_header_len + dev->needed_headroom) < 0)

Should we use LL_RESERVED_SPACE() here?

Thanks
Hangbin
> +			return -ENOMEM;
>  		vhdr = skb_push(skb, VLAN_HLEN);
>  
>  		vlan_tci = vlan->vlan_id;
> -- 
> 2.55.0.1082.g2b9226bbc0-goog
> 

Re: [PATCH net] vlan: ensure sufficient headroom in vlan_dev_hard_header()

[Eric Dumazet]

On Thu, Sep 24, 2026 at 3:48 AM Hangbin Liu <hangbin.liu@linux.dev> wrote:
>
> Hi Eric,
> On Tue, Sep 22, 2026 at 11:39:24AM +0000, Eric Dumazet wrote:
> > Callers that only reserve ETH_HLEN or less (such as llc_alloc_frame() or
> > br_send_bpdu()), or skbs allocated before dynamic device/headroom
> > changes (e.g. toggling VLAN_FLAG_REORDER_HDR or bonding/team switching
> > slaves), can reach vlan_dev_hard_header() with insufficient headroom and
> > trigger skb_under_panic().
> >
> > Use skb_cow_head() in vlan_dev_hard_header() when VLAN_FLAG_REORDER_HDR
> > is not set to ensure sufficient headroom for the VLAN header(s) and the
> > underlying device hard header.
> >
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Reported-by: Zixuan Chai <petalzu987@gmail.com>
> > Closes: https://lore.kernel.org/netdev/cover.1789987105.git.petalzu987@gmail.com/
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > ---
> >  net/8021q/vlan_dev.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> > index 2859cbac3f266b7c4e3f44f41280d33ab69c5270..3598ad0779f050d25c6110de476fe9613137f983 100644
> > --- a/net/8021q/vlan_dev.c
> > +++ b/net/8021q/vlan_dev.c
> > @@ -55,6 +55,9 @@ static int vlan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
> >       int rc;
> >
> >       if (!(vlan->flags & VLAN_FLAG_REORDER_HDR)) {
> > +             if (skb_cow_head(skb,
> > +                              dev->hard_header_len + dev->needed_headroom) < 0)
>
> Should we use LL_RESERVED_SPACE() here?

I think using LL_RESERVED_SPACE() would force a reallocation in some cases.

In V2 I will however add READ_ONCE() to comply with the bot findings.

Re: [PATCH net] vlan: ensure sufficient headroom in vlan_dev_hard_header()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 5 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 3 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The patch makes vlan_dev_hard_header() return -ENOMEM for
  ARPHRD_ETHER-backed VLAN devices for the first time, which activates a…
- [Medium] vlan_dev_hard_header() (net/8021q/vlan_dev.c:58-60) now
  returns -ENOMEM *before* pushing anything, making…
- [Medium] sch_teql swallows the newly introduced VLAN header-
  construction failure and selects its 'transmit' branch.
- [Medium] The new -ENOMEM return from vlan_dev_hard_header() makes two
  LLC SAP response builders leak their response skb.
- [Low] The new headroom computation in vlan_dev_hard_header()
  (net/8021q/vlan_dev.c:58-59) reads dev->needed_headroom with a plain…

--

vlan: ensure sufficient headroom in vlan_dev_hard_header()

This adds an skb_cow_head() call to vlan_dev_hard_header() when
VLAN_FLAG_REORDER_HDR is clear, so that callers which reserved only
ETH_HLEN (or skbs allocated before a headroom change) no longer hit
skb_under_panic() when the VLAN header is pushed.

Closes: https://lore.kernel.org/netdev/cover.1789987105.git.petalzu987@gmail.com/

> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index 2859cbac3f266..3598ad0779f05 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -55,6 +55,9 @@ static int vlan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
>  	int rc;
>  
>  	if (!(vlan->flags & VLAN_FLAG_REORDER_HDR)) {
> +		if (skb_cow_head(skb,
> +				 dev->hard_header_len + dev->needed_headroom) < 0)
> +			return -ENOMEM;

[Severity: High]
Can this new -ENOMEM return wake up the error path in
llc_conn_ac_resend_i_xxx_x_set_0_or_send_rr() in net/llc/llc_c_ac.c?

That action frees the wrong skb when llc_mac_hdr_init() fails:

net/llc/llc_c_ac.c:llc_conn_ac_resend_i_xxx_x_set_0_or_send_rr() {
	struct llc_pdu_sn *pdu = llc_pdu_sn_hdr(skb);
	...
	struct sk_buff *nskb = llc_alloc_frame(sk, llc->dev, LLC_PDU_TYPE_U, 0);

	if (nskb) {
		...
		rc = llc_mac_hdr_init(nskb, llc->dev->dev_addr, llc->daddr.mac);
		if (likely(!rc))
			llc_conn_send_pdu(sk, nskb);
		else
			kfree_skb(skb);
	}
	if (rc) {
		nr = LLC_I_GET_NR(pdu);
	...
}

The sibling actions in the same file use kfree_skb(nskb) here. With
kfree_skb(skb), does nskb leak while the caller-owned event skb is put
twice, given that llc_conn_state_process() documents "This function
always consumes a reference to the skb" and does kfree_skb(skb) at
out_skb_put?  And is the following LLC_I_GET_NR(pdu) then a read of the
freed skb, since pdu points into it?

The failure branch looks genuinely reachable now: llc_alloc_frame()
reserves 3 + llc_mac_header_len(ARPHRD_ETHER) = 17 bytes, the LLC PDU
push leaves 14 bytes of headroom, so on a VLAN device with
VLAN_FLAG_REORDER_HDR clear skb_cow_head(skb, 14 + 18) always calls
pskb_expand_head(GFP_ATOMIC) and can fail under memory pressure.

Before this change the branch appears to have been dead code for
Ethernet-backed devices: eth_header() pushes first and only returns a
negative value when daddr is NULL, and dev_hard_header() returns 0 when
there are no header_ops.

[Severity: Medium]
Along the same lines, do llc_sap_action_send_xid_r() and
llc_sap_action_send_test_r() in net/llc/llc_s_ac.c leak their response
skb now?

net/llc/llc_s_ac.c:llc_sap_action_send_xid_r() {
	nskb = llc_alloc_frame(NULL, skb->dev, LLC_PDU_TYPE_U,
			       sizeof(struct llc_xid_info));
	...
	rc = llc_mac_hdr_init(nskb, mac_sa, mac_da);
	if (likely(!rc))
		rc = dev_queue_xmit(nskb);
out:
	return rc;
}

On the new -ENOMEM, nskb is neither queued nor freed, and the pointer is
local so no caller can release it.  Both actions run on received XID and
TEST command PDUs, and the skb from llc_alloc_frame() has only 14 bytes
of headroom, so the cow is always attempted.

[Severity: Medium]
A related question about the new failure mode in general: this is the
first time dev_hard_header() can return a negative value for an
ARPHRD_ETHER-backed VLAN device with a non-NULL daddr, and it now means
"nothing was pushed" rather than "header pushed, but tell me something".
Several callers discard the return and transmit immediately:

net/bridge/br_stp_bpdu.c:br_send_bpdu()
	llc_mac_hdr_init(skb, p->dev->dev_addr, p->br->group_addr);
	skb_reset_mac_header(skb);
	NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_OUT, ... br_send_bpdu_finish);

net/tipc/bearer.c:tipc_l2_send_msg()
	dev_hard_header(skb, dev, ETH_P_TIPC, dest->value, dev->dev_addr, skb->len);
	dev_queue_xmit(skb);

net/netfilter/nft_reject_netdev.c
	dev_hard_header(nskb, nskb->dev, ntohs(oldskb->protocol), ...);
	dev_queue_xmit(nskb);

Same pattern in the direct-xmit path of net/netfilter/nf_flow_table_ip.c
and in drivers/net/ppp/pppoe.c, where the skb is reserved with only
dev->hard_header_len so the cow is always attempted.

When skb_cow_head() fails, is the skb handed to dev_queue_xmit() with
skb->data still at the payload and no VLAN or Ethernet header, so that
vlan_dev_hard_start_xmit() reads payload bytes as struct vlan_ethhdr and
a frame with a garbage link header goes on the wire?  br_send_bpdu() is
one of the two callers named in the commit message.

[Severity: Medium]
Is the sch_teql path also affected?  __teql_resolve() in
net/sched/sch_teql.c shadows its return variable:

net/sched/sch_teql.c:__teql_resolve() {
	int err = 0;
	...
	if (neigh_event_send(n, skb_res) == 0) {
		int err;
		char haddr[MAX_ADDR_LEN];

		neigh_ha_snapshot(haddr, n, dev);
		err = dev_hard_header(skb, dev, ntohs(skb_protocol(skb, false)),
				      haddr, NULL, skb->len);

		if (err < 0)
			err = -EINVAL;
	} else {
	...
	return err;
}

The inner err holds the failure, so 0 is returned and teql_master_xmit()
takes "case 0" and calls netdev_start_xmit() on the slave.  For the
negative returns that existed before (for example ipgre_header(), which
returns negative after pushing its header) that was harmless, but with
the new -ENOMEM nothing has been pushed.  Should the inner declaration
be dropped, or the return checked by the caller?

[Severity: Low]
This isn't a bug, but should dev->needed_headroom be read with
READ_ONCE() here?  Every other in-tree reader annotates it, for example:

include/linux/netdevice.h
	#define LL_RESERVED_SPACE_EX(dev, hlen) \
		((((hlen) + READ_ONCE((dev)->needed_headroom)) \
		  & ~(HH_DATA_MOD - 1)) + HH_DATA_MOD)

The writer is vlan_transfer_features(), which runs from the VLAN work
item under RTNL:

net/8021q/vlan_dev.c:vlan_transfer_features()
	vlandev->needed_headroom = dev->needed_headroom + VLAN_HLEN;
	...
	vlandev->hard_header_len = dev->hard_header_len;

while vlan_dev_hard_header() runs from the transmit path without RTNL.

>  		vhdr = skb_push(skb, VLAN_HLEN);
>  
>  		vlan_tci = vlan->vlan_id;

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260922113924.3165190-1-edumazet%40google.com

Re: [PATCH net] vlan: ensure sufficient headroom in vlan_dev_hard_header()

[Eric Dumazet]

On Thu, Sep 24, 2026 at 7:41 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 5 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · Medium: 3 · Low: 1
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [High] The patch makes vlan_dev_hard_header() return -ENOMEM for
>   ARPHRD_ETHER-backed VLAN devices for the first time, which activates a…
> - [Medium] vlan_dev_hard_header() (net/8021q/vlan_dev.c:58-60) now
>   returns -ENOMEM *before* pushing anything, making…
> - [Medium] sch_teql swallows the newly introduced VLAN header-
>   construction failure and selects its 'transmit' branch.
> - [Medium] The new -ENOMEM return from vlan_dev_hard_header() makes two
>   LLC SAP response builders leak their response skb.
> - [Low] The new headroom computation in vlan_dev_hard_header()
>   (net/8021q/vlan_dev.c:58-59) reads dev->needed_headroom with a plain…
>
> --
>
> vlan: ensure sufficient headroom in vlan_dev_hard_header()
>
> This adds an skb_cow_head() call to vlan_dev_hard_header() when
> VLAN_FLAG_REORDER_HDR is clear, so that callers which reserved only
> ETH_HLEN (or skbs allocated before a headroom change) no longer hit
> skb_under_panic() when the VLAN header is pushed.
>
> Closes: https://lore.kernel.org/netdev/cover.1789987105.git.petalzu987@gmail.com/
>
> > diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> > index 2859cbac3f266..3598ad0779f05 100644
> > --- a/net/8021q/vlan_dev.c
> > +++ b/net/8021q/vlan_dev.c
> > @@ -55,6 +55,9 @@ static int vlan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
> >       int rc;
> >
> >       if (!(vlan->flags & VLAN_FLAG_REORDER_HDR)) {
> > +             if (skb_cow_head(skb,
> > +                              dev->hard_header_len + dev->needed_headroom) < 0)
> > +                     return -ENOMEM;
>
> [Severity: High]
> Can this new -ENOMEM return wake up the error path in
> llc_conn_ac_resend_i_xxx_x_set_0_or_send_rr() in net/llc/llc_c_ac.c?
>
> That action frees the wrong skb when llc_mac_hdr_init() fails:
>
> net/llc/llc_c_ac.c:llc_conn_ac_resend_i_xxx_x_set_0_or_send_rr() {
>         struct llc_pdu_sn *pdu = llc_pdu_sn_hdr(skb);
>         ...
>         struct sk_buff *nskb = llc_alloc_frame(sk, llc->dev, LLC_PDU_TYPE_U, 0);
>
>         if (nskb) {
>                 ...
>                 rc = llc_mac_hdr_init(nskb, llc->dev->dev_addr, llc->daddr.mac);
>                 if (likely(!rc))
>                         llc_conn_send_pdu(sk, nskb);
>                 else
>                         kfree_skb(skb);
>         }
>         if (rc) {
>                 nr = LLC_I_GET_NR(pdu);
>         ...
> }
>
> The sibling actions in the same file use kfree_skb(nskb) here. With
> kfree_skb(skb), does nskb leak while the caller-owned event skb is put
> twice, given that llc_conn_state_process() documents "This function
> always consumes a reference to the skb" and does kfree_skb(skb) at
> out_skb_put?  And is the following LLC_I_GET_NR(pdu) then a read of the
> freed skb, since pdu points into it?
>
> The failure branch looks genuinely reachable now: llc_alloc_frame()
> reserves 3 + llc_mac_header_len(ARPHRD_ETHER) = 17 bytes, the LLC PDU
> push leaves 14 bytes of headroom, so on a VLAN device with
> VLAN_FLAG_REORDER_HDR clear skb_cow_head(skb, 14 + 18) always calls
> pskb_expand_head(GFP_ATOMIC) and can fail under memory pressure.
>

1. With the companion patch "[PATCH net v2 1/1] llc: reserve device headroom
   for allocated frames"
   (https://lore.kernel.org/netdev/20260924012613.2533934-1-weir@nebusec.ai/),
   llc_alloc_frame() reserves LL_RESERVED_SPACE(dev) (32 bytes for a VLAN
   device), so skb_cow_head() in vlan_dev_hard_header() is a no-op on the
   normal path and only triggers on dynamic headroom/device changes.

2. The typo in llc_conn_ac_resend_i_xxx_x_set_0_or_send_rr() (calling
   kfree_skb(skb) instead of kfree_skb(nskb)) is a pre-existing bug in
   net/llc/llc_c_ac.c (already reachable if dev->type changes and
   llc_mac_hdr_init() returns -EINVAL), which can be fixed separately in LLC.

> Before this change the branch appears to have been dead code for
> Ethernet-backed devices: eth_header() pushes first and only returns a
> negative value when daddr is NULL, and dev_hard_header() returns 0 when
> there are no header_ops.
>
> [Severity: Medium]
> Along the same lines, do llc_sap_action_send_xid_r() and
> llc_sap_action_send_test_r() in net/llc/llc_s_ac.c leak their response
> skb now?
>

Same as above: llc_alloc_frame() reserves LL_RESERVED_SPACE(dev) with the
companion LLC patch, and the missing kfree_skb(nskb) on llc_mac_hdr_init()
error in llc_sap_action_send_{xid,test}_r() is a pre-existing bug in
net/llc/llc_s_ac.c.

> net/llc/llc_s_ac.c:llc_sap_action_send_xid_r() {
>         nskb = llc_alloc_frame(NULL, skb->dev, LLC_PDU_TYPE_U,
>                                sizeof(struct llc_xid_info));
>         ...
>         rc = llc_mac_hdr_init(nskb, mac_sa, mac_da);
>         if (likely(!rc))
>                 rc = dev_queue_xmit(nskb);
> out:
>         return rc;
> }
>
> On the new -ENOMEM, nskb is neither queued nor freed, and the pointer is
> local so no caller can release it.  Both actions run on received XID and
> TEST command PDUs, and the skb from llc_alloc_frame() has only 14 bytes
> of headroom, so the cow is always attempted.
>
> [Severity: Medium]
> A related question about the new failure mode in general: this is the
> first time dev_hard_header() can return a negative value for an
> ARPHRD_ETHER-backed VLAN device with a non-NULL daddr, and it now means
> "nothing was pushed" rather than "header pushed, but tell me something".
> Several callers discard the return and transmit immediately:
>
> net/bridge/br_stp_bpdu.c:br_send_bpdu()
>         llc_mac_hdr_init(skb, p->dev->dev_addr, p->br->group_addr);
>         skb_reset_mac_header(skb);
>         NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_OUT, ... br_send_bpdu_finish);
>
> net/tipc/bearer.c:tipc_l2_send_msg()
>         dev_hard_header(skb, dev, ETH_P_TIPC, dest->value, dev->dev_addr, skb->len);
>         dev_queue_xmit(skb);
>
> net/netfilter/nft_reject_netdev.c
>         dev_hard_header(nskb, nskb->dev, ntohs(oldskb->protocol), ...);
>         dev_queue_xmit(nskb);
>
> Same pattern in the direct-xmit path of net/netfilter/nf_flow_table_ip.c
> and in drivers/net/ppp/pppoe.c, where the skb is reserved with only
> dev->hard_header_len so the cow is always attempted.

This analysis is incorrect about the headroom reserved by these callers:
- drivers/net/ppp/pppoe.c already reserves LL_RESERVED_SPACE(dev) in both
  pppoe_sendmsg() (hlen = LL_RESERVED_SPACE(dev)) and pppoe_xmit()
  (skb_cow_head(skb, LL_RESERVED_SPACE(dev) + sizeof(*ph))).
- __nf_flow_queue_xmit() in net/netfilter/nf_flow_table_ip.c already calls
  skb_expand_head(skb, LL_RESERVED_SPACE(dev)) immediately before calling
  dev_hard_header().
- br_send_bpdu() uses dev_alloc_skb(length + LLC_RESERVE), which reserves
  NET_SKB_PAD (64 bytes on x86_64) + LLC_RESERVE, leaving 64 bytes of
  headroom after the 3-byte LLC header.
- nft_reject_netdev.c and tipc_buf_acquire() both reserve LL_MAX_HEADER.

>
> When skb_cow_head() fails, is the skb handed to dev_queue_xmit() with
> skb->data still at the payload and no VLAN or Ethernet header, so that
> vlan_dev_hard_start_xmit() reads payload bytes as struct vlan_ethhdr and
> a frame with a garbage link header goes on the wire?  br_send_bpdu() is
> one of the two callers named in the commit message.
>
> [Severity: Medium]
> Is the sch_teql path also affected?  __teql_resolve() in
> net/sched/sch_teql.c shadows its return variable:

The inner 'int err;' shadowing the outer 'err' in __teql_resolve() is a
pre-existing bug in net/sched/sch_teql.c unrelated to this change.

>
> net/sched/sch_teql.c:__teql_resolve() {
>         int err = 0;
>         ...
>         if (neigh_event_send(n, skb_res) == 0) {
>                 int err;
>                 char haddr[MAX_ADDR_LEN];
>
>                 neigh_ha_snapshot(haddr, n, dev);
>                 err = dev_hard_header(skb, dev, ntohs(skb_protocol(skb, false)),
>                                       haddr, NULL, skb->len);
>
>                 if (err < 0)
>                         err = -EINVAL;
>         } else {
>         ...
>         return err;
> }
>
> The inner err holds the failure, so 0 is returned and teql_master_xmit()
> takes "case 0" and calls netdev_start_xmit() on the slave.  For the
> negative returns that existed before (for example ipgre_header(), which
> returns negative after pushing its header) that was harmless, but with
> the new -ENOMEM nothing has been pushed.  Should the inner declaration
> be dropped, or the return checked by the caller?
>
> [Severity: Low]
> This isn't a bug, but should dev->needed_headroom be read with
> READ_ONCE() here?  Every other in-tree reader annotates it, for example:

Yes I can add this in V2.

READ_ONCE(dev->needed_headroom) (and READ_ONCE(dev->hard_header_len)):

pw-bot: cr