[PATCH net] tun: prevent underflow in rx headroom calculation

[PATCH net] tun: prevent underflow in rx headroom calculation

[Asim Viladi Oglu Manizada]

tun_get_user() calculates good_linear with SKB_MAX_HEAD(align). The align
value comes from ndo_set_rx_headroom() and can be larger than the linear
space available in a one-page skb head.

OVS can reach this case by carrying headroom from a netkit/VXLAN port to a
TUN port. SKB_MAX_HEAD() then underflows, leaving good_linear negative.
Assigning that value to the size_t linear variable in tun_get_user()
converts it to a large positive value. The wrapped value is passed to
tun_alloc_skb(), where prepad + linear and len - linear wrap. skb->data can
then end up past the allocated head, and later packet processing can access
memory outside the skb.

Clamp good_linear to zero when SKB_MAX_HEAD() returns a negative value.
This lets tun_alloc_skb() allocate the requested headroom and place the
packet data linearly or in fragments without wrapping.

Fixes: eaea34b23c46 ("net/tun: implement ndo_set_rx_headroom")
Cc: stable@vger.kernel.org
Assisted-by: avom-custom-harness:gpt-5.5-qwen3.6-mod-mix
Signed-off-by: Asim Viladi Oglu Manizada <manizada@pm.me>
---
 drivers/net/tun.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index ffbe6f13fb1..d3be0f2d5df 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1780,6 +1780,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 	}
 
 	good_linear = SKB_MAX_HEAD(align);
+	good_linear = max(good_linear, 0);
 
 	if (msg_control) {
 		struct iov_iter i = *from;
-- 
2.39.5

Re: [PATCH net] tun: prevent underflow in rx headroom calculation

[Willem de Bruijn]

Asim Viladi Oglu Manizada wrote:
> tun_get_user() calculates good_linear with SKB_MAX_HEAD(align). The align
> value comes from ndo_set_rx_headroom() and can be larger than the linear
> space available in a one-page skb head.
> 
> OVS can reach this case by carrying headroom from a netkit/VXLAN port to a
> TUN port. SKB_MAX_HEAD() then underflows, leaving good_linear negative.

Separate from protecting tun from such huge headroom, which is good:

Is cooking paths with such huge headroom itself incorrect? Should
netkit have some reasonable upper bound on configurable headroom?
Netkit is not the only path that can reach this, right?

> Assigning that value to the size_t linear variable in tun_get_user()
> converts it to a large positive value. The wrapped value is passed to
> tun_alloc_skb(), where prepad + linear and len - linear wrap. skb->data can
> then end up past the allocated head, and later packet processing can access
> memory outside the skb.
> 
> Clamp good_linear to zero when SKB_MAX_HEAD() returns a negative value.
> This lets tun_alloc_skb() allocate the requested headroom and place the
> packet data linearly or in fragments without wrapping.
> 
> Fixes: eaea34b23c46 ("net/tun: implement ndo_set_rx_headroom")
> Cc: stable@vger.kernel.org
> Assisted-by: avom-custom-harness:gpt-5.5-qwen3.6-mod-mix
> Signed-off-by: Asim Viladi Oglu Manizada <manizada@pm.me>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Re: [PATCH net] tun: prevent underflow in rx headroom calculation

[manizada]

On Tuesday, July 21st, 2026 at 12:59 AM, Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:

> Asim Viladi Oglu Manizada wrote:
> > tun_get_user() calculates good_linear with SKB_MAX_HEAD(align). The align
> > value comes from ndo_set_rx_headroom() and can be larger than the linear
> > space available in a one-page skb head.
> >
> > OVS can reach this case by carrying headroom from a netkit/VXLAN port to a
> > TUN port. SKB_MAX_HEAD() then underflows, leaving good_linear negative.
> 
> Separate from protecting tun from such huge headroom, which is good:
> 
> Is cooking paths with such huge headroom itself incorrect? Should
> netkit have some reasonable upper bound on configurable headroom?
> Netkit is not the only path that can reach this, right?

Thanks for the review. Yep, oversized headroom can reach TUN/TAP without netkit via OVS/veth/VXLAN, OVS/veth/GRETAP, and bridge/veth/GRETAP/TAP (at least on stock Ubuntu 6.8). A netkit-side limit might still be good too, separate from these other paths, for defense in depth.

> 
> > Assigning that value to the size_t linear variable in tun_get_user()
> > converts it to a large positive value. The wrapped value is passed to
> > tun_alloc_skb(), where prepad + linear and len - linear wrap. skb->data can
> > then end up past the allocated head, and later packet processing can access
> > memory outside the skb.
> >
> > Clamp good_linear to zero when SKB_MAX_HEAD() returns a negative value.
> > This lets tun_alloc_skb() allocate the requested headroom and place the
> > packet data linearly or in fragments without wrapping.
> >
> > Fixes: eaea34b23c46 ("net/tun: implement ndo_set_rx_headroom")
> > Cc: stable@vger.kernel.org
> > Assisted-by: avom-custom-harness:gpt-5.5-qwen3.6-mod-mix
> > Signed-off-by: Asim Viladi Oglu Manizada <manizada@pm.me>
> 
> Reviewed-by: Willem de Bruijn <willemb@google.com>
>

Re: [PATCH net] tun: prevent underflow in rx headroom calculation

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
tun: prevent underflow in rx headroom calculation

This patch clamps good_linear to zero in tun_get_user() so that a large
tun->align value coming from ndo_set_rx_headroom() does not cause
SKB_MAX_HEAD(align) to underflow into a huge size_t when assigned to the
linear variable, and later cause the skb data pointer to end up outside
the allocated head.

> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index ffbe6f13fb1f..d3be0f2d5df8 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1780,6 +1780,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  	}
>  
>  	good_linear = SKB_MAX_HEAD(align);
> +	good_linear = max(good_linear, 0);

[High]
Does this fully cover the IFF_TAP path when align is large enough that
SKB_MAX_HEAD(align) is <= 0?

With the clamp, good_linear becomes 0, and later in tun_get_user() the
non-zerocopy path does:

    copylen = len;
    linear = min(hdr_len, good_linear);
    ...
    if (!linear)
        linear = min_t(size_t, good_linear, copylen);
    skb = tun_alloc_skb(tfile, align, copylen, linear, noblock);

so linear ends up 0 and the resulting skb has skb_headlen(skb) == 0.

For IFF_TAP without IFF_NAPI_FRAGS the code then calls:

    skb->protocol = eth_type_trans(skb, tun->dev);

eth_type_trans() -> eth_skb_pull_mac() -> __skb_pull(skb, ETH_HLEN),
and __skb_pull() has:

    if (unlikely(skb->len < skb->data_len))
        BUG();

Since skb->len and skb->data_len both start at the packet length and
skb->len is decremented by ETH_HLEN in the pull, would this reach the
BUG() when a user writes to /dev/net/tun with tun->align pushed above
the SKB_MAX_HEAD threshold via the OVS/netkit/VXLAN stacking described
in the commit message?

The pre-existing behavior at align == 3776 appears to hit the same
BUG() and align > 3776 pre-patch corrupted memory before reaching it,
so this change is an improvement, but would forcing linear >= ETH_HLEN
in the IFF_TAP path (or calling pskb_may_pull() before eth_type_trans()
regardless of the frags flag) be needed to close the remaining panic?
-- 
pw-bot: cr

Re: [PATCH net] tun: prevent underflow in rx headroom calculation

[manizada]

On Thursday, July 23rd, 2026 at 10:14 AM, Jakub Kicinski <kuba@kernel.org> wrote:

> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> tun: prevent underflow in rx headroom calculation
> 
> This patch clamps good_linear to zero in tun_get_user() so that a large
> tun->align value coming from ndo_set_rx_headroom() does not cause
> SKB_MAX_HEAD(align) to underflow into a huge size_t when assigned to the
> linear variable, and later cause the skb data pointer to end up outside
> the allocated head.
> 
> > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > index ffbe6f13fb1f..d3be0f2d5df8 100644
> > --- a/drivers/net/tun.c
> > +++ b/drivers/net/tun.c
> > @@ -1780,6 +1780,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> >  	}
> >
> >  	good_linear = SKB_MAX_HEAD(align);
> > +	good_linear = max(good_linear, 0);
> 
> [High]
> Does this fully cover the IFF_TAP path when align is large enough that
> SKB_MAX_HEAD(align) is <= 0?
> 
> With the clamp, good_linear becomes 0, and later in tun_get_user() the
> non-zerocopy path does:
> 
>     copylen = len;
>     linear = min(hdr_len, good_linear);
>     ...
>     if (!linear)
>         linear = min_t(size_t, good_linear, copylen);
>     skb = tun_alloc_skb(tfile, align, copylen, linear, noblock);
> 
> so linear ends up 0 and the resulting skb has skb_headlen(skb) == 0.
> 
> For IFF_TAP without IFF_NAPI_FRAGS the code then calls:
> 
>     skb->protocol = eth_type_trans(skb, tun->dev);
> 
> eth_type_trans() -> eth_skb_pull_mac() -> __skb_pull(skb, ETH_HLEN),
> and __skb_pull() has:
> 
>     if (unlikely(skb->len < skb->data_len))
>         BUG();
> 
> Since skb->len and skb->data_len both start at the packet length and
> skb->len is decremented by ETH_HLEN in the pull, would this reach the
> BUG() when a user writes to /dev/net/tun with tun->align pushed above
> the SKB_MAX_HEAD threshold via the OVS/netkit/VXLAN stacking described
> in the commit message?
> 
> The pre-existing behavior at align == 3776 appears to hit the same
> BUG() and align > 3776 pre-patch corrupted memory before reaching it,
> so this change is an improvement, but would forcing linear >= ETH_HLEN
> in the IFF_TAP path (or calling pskb_may_pull() before eth_type_trans()
> regardless of the frags flag) be needed to close the remaining panic?

Oh this is a good catch. But while making the TAP pskb_may_pull
(ETH_HLEN) check unconditional would address the eth_type_trans() case,
raw TUN with IFF_NO_PI also directly reads the first protocol byte from
skb->data, so we'd have the same issue there.

Rather than add consumer-side handling for a fully nonlinear skb state
introduced by the repair, maybe the cleanest is preventing TUN from
creating this state?

	#define TUN_MAX_HEADROOM 512

	tun->align = clamp(new_hr, NET_SKB_PAD, TUN_MAX_HEADROOM);

This fixes the original SKB_MAX_HEAD() arithmetic issue, and also preserves
linear space for both the raw-TUN protocol byte and the TAP Ethernet
header. The 512-byte value matches the existing ceiling in
ip_tunnel_adj_headroom(), so requests above the cap can require later
skb expansion instead of unsafe preallocation by TUN.

Does this sound reasonable? If so, I can post v2 using the TUN-side cap.


> --
> pw-bot: cr
>

Re: [PATCH net] tun: prevent underflow in rx headroom calculation

[Willem de Bruijn]

manizada wrote:
> 
> 
> 
> 
> On Thursday, July 23rd, 2026 at 10:14 AM, Jakub Kicinski <kuba@kernel.org> wrote:
> 
> > This is an AI-generated review of your patch. The human sending this
> > email has considered the AI review valid, or at least plausible.
> > ---
> > tun: prevent underflow in rx headroom calculation
> > 
> > This patch clamps good_linear to zero in tun_get_user() so that a large
> > tun->align value coming from ndo_set_rx_headroom() does not cause
> > SKB_MAX_HEAD(align) to underflow into a huge size_t when assigned to the
> > linear variable, and later cause the skb data pointer to end up outside
> > the allocated head.
> > 
> > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > > index ffbe6f13fb1f..d3be0f2d5df8 100644
> > > --- a/drivers/net/tun.c
> > > +++ b/drivers/net/tun.c
> > > @@ -1780,6 +1780,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> > >  	}
> > >
> > >  	good_linear = SKB_MAX_HEAD(align);
> > > +	good_linear = max(good_linear, 0);
> > 
> > [High]
> > Does this fully cover the IFF_TAP path when align is large enough that
> > SKB_MAX_HEAD(align) is <= 0?
> > 
> > With the clamp, good_linear becomes 0, and later in tun_get_user() the
> > non-zerocopy path does:
> > 
> >     copylen = len;
> >     linear = min(hdr_len, good_linear);
> >     ...
> >     if (!linear)
> >         linear = min_t(size_t, good_linear, copylen);
> >     skb = tun_alloc_skb(tfile, align, copylen, linear, noblock);
> > 
> > so linear ends up 0 and the resulting skb has skb_headlen(skb) == 0.
> > 
> > For IFF_TAP without IFF_NAPI_FRAGS the code then calls:
> > 
> >     skb->protocol = eth_type_trans(skb, tun->dev);
> > 
> > eth_type_trans() -> eth_skb_pull_mac() -> __skb_pull(skb, ETH_HLEN),
> > and __skb_pull() has:
> > 
> >     if (unlikely(skb->len < skb->data_len))
> >         BUG();
> > 
> > Since skb->len and skb->data_len both start at the packet length and
> > skb->len is decremented by ETH_HLEN in the pull, would this reach the
> > BUG() when a user writes to /dev/net/tun with tun->align pushed above
> > the SKB_MAX_HEAD threshold via the OVS/netkit/VXLAN stacking described
> > in the commit message?
> > 
> > The pre-existing behavior at align == 3776 appears to hit the same
> > BUG() and align > 3776 pre-patch corrupted memory before reaching it,
> > so this change is an improvement, but would forcing linear >= ETH_HLEN
> > in the IFF_TAP path (or calling pskb_may_pull() before eth_type_trans()
> > regardless of the frags flag) be needed to close the remaining panic?
> 
> Oh this is a good catch. But while making the TAP pskb_may_pull
> (ETH_HLEN) check unconditional would address the eth_type_trans() case,
> raw TUN with IFF_NO_PI also directly reads the first protocol byte from
> skb->data, so we'd have the same issue there.
> 
> Rather than add consumer-side handling for a fully nonlinear skb state
> introduced by the repair, maybe the cleanest is preventing TUN from
> creating this state?
> 
> 	#define TUN_MAX_HEADROOM 512
> 
> 	tun->align = clamp(new_hr, NET_SKB_PAD, TUN_MAX_HEADROOM);
> 
> This fixes the original SKB_MAX_HEAD() arithmetic issue, and also preserves
> linear space for both the raw-TUN protocol byte and the TAP Ethernet
> header. The 512-byte value matches the existing ceiling in
> ip_tunnel_adj_headroom(), so requests above the cap can require later

Is that a ceiling specific to ip_tunnel or universal for tuntap? I
suspect only the second. In which case this would add a new condition.

> skb expansion instead of unsafe preallocation by TUN.
> 
> Does this sound reasonable? If so, I can post v2 using the TUN-side cap.

Why NET_SKB_PAD, aside from that it happens to be larger than both
IFF_TAP and IFF_NO_PI cases?  (good catch on that NO_PI btw.)

The IFF_TAP case already has a pskb_may_pull, but only for frags.
Perhaps we should just always enable that. And a similar check before
the NO_PI case reads skb->data[0]. That is in line with standard rx
protocol parsing logic.

Alternatively indeed clamp, but to the true minimum required values in
these cases, which coincide with the values tested in pskb_may_pull.