Re: [PATCH net v2] net: tun: bound receive headroom

On Wednesday, August 5th, 2026 at 6:14 AM, Willem de Bruijn <[email protected]> wrote:

> Asim Viladi Oglu Manizada wrote:
> > tun_get_user() uses tun->align both as skb headroom and when choosing how
> > much packet data to keep linear. OVS can propagate an oversized headroom
> > request from another port to TUN or TAP.
> >
> > When align is larger than the usable space in a one-page skb head,
> > SKB_MAX_HEAD(align) underflows and the result becomes negative when stored
> > in good_linear. That value later wraps when assigned to the size_t linear
> > variable, and tun_alloc_skb() can place skb->data outside the allocated
> > head.
> >
> > Bound the headroom stored by TUN to the one-page skb-head budget and the
> > largest non-sentinel 16-bit skb header offset. Leave one linear byte for
> > raw TUN and a complete Ethernet header for TAP, including NET_IP_ALIGN.
> >
> > Also pull the raw-TUN protocol byte and the TAP Ethernet header before
> > accessing them, so these checks remain safe for nonlinear skbs supplied by
> > other allocation paths.
> >
> > Fixes: eaea34b23c46 ("net/tun: implement ndo_set_rx_headroom")
> > Cc: [email protected]
> > Assisted-by: avom-custom-harness:gpt-5.5-qwen3.6-mod-mix
> > Signed-off-by: Asim Viladi Oglu Manizada <[email protected]>
> > ---
> > v2:
> > - bound tun->align instead of clamping good_linear to zero
> > - derive the bound from the one-page head, 16-bit offset, and TUN/TAP
> >   linear-header requirements
> > - pull the raw-TUN protocol byte before reading it
> > - make the TAP Ethernet-header pull unconditional
> > v1: https://lore.kernel.org/netdev/[email protected]/
> >
> >  drivers/net/tun.c | 21 ++++++++++++++++-----
> >  1 file changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > index fed9dfdfcc3b..efd2e7d75c9a 100644
> > --- a/drivers/net/tun.c
> > +++ b/drivers/net/tun.c
> > @@ -1107,11 +1107,16 @@ static netdev_features_t tun_net_fix_features(struct net_device *dev,
> >  static void tun_set_headroom(struct net_device *dev, int new_hr)
> >  {
> >  	struct tun_struct *tun = netdev_priv(dev);
> > +	size_t max_headroom;
> >
> > -	if (new_hr < NET_SKB_PAD)
> > -		new_hr = NET_SKB_PAD;
> > +	max_headroom = min_t(size_t, SKB_MAX_HEAD(0), U16_MAX - 1);
> >
> > -	tun->align = new_hr;
> > +	if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP)
> > +		max_headroom -= ETH_HLEN + NET_IP_ALIGN;
> > +	else
> > +		max_headroom -= 1;
> > +
> > +	tun->align = clamp_t(int, new_hr, NET_SKB_PAD, max_headroom);
> >  }
> >
> >  static void
> > @@ -1822,7 +1827,13 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> >  	switch (tun->flags & TUN_TYPE_MASK) {
> >  	case IFF_TUN:
> >  		if (tun->flags & IFF_NO_PI) {
> > -			u8 ip_version = skb->len ? (skb->data[0] >> 4) : 0;
> > +			u8 ip_version;
> > +
> > +			if (skb->len && !pskb_may_pull(skb, 1)) {
> > +				err = -ENOMEM;
> > +				goto drop;
> > +			}
> > +			ip_version = skb->len ? (skb->data[0] >> 4) : 0;
> 
> Overall, LGTM, thanks. Let's wait for the bots too.
> 
> This can be a bit simpler. pskb_may_pull checks that len < skb->len.
> And it is not an allocation failure, but a bad packet.
> 
> 	if (!pskb_may_pull(skb, 1)) {
> 		err = -EINVAL;
> 		goto drop;
> 	}
> 
> 	ip_version = skb->data[0] >> 4;

Makes sense, I'll do that in v3, it also keeps returning -EINVAL
in the empty packet case without needing a skb->len check. Will
resubmit within 72 hours or so unless I hear any more concerns.
I'll respond to the AI review separately, those things don't look
concerning.