mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Dongli Zhang <dongli.zhang@oracle.com>
To: Weiming Shi <bestswngs@gmail.com>,
	Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: netdev@vger.kernel.org, jasowang@redhat.com, pabeni@redhat.com,
	kuba@kernel.org, edumazet@google.com, xmei5@asu.edu,
	linux-kernel@vger.kernel.org, Si-Wei Liu <si-wei.liu@oracle.com>,
	mst@redhat.com
Subject: Re: [PATCH net] tun: free page on short-frame rejection in tun_xdp_one()
Date: Thu, 21 May 2026 14:01:39 -0700	[thread overview]
Message-ID: <cc1ea6d3-f694-481c-a5db-4356c5076207@oracle.com> (raw)
In-Reply-To: <ag80f7ENWAHsK-fw@Air.local>



On 2026-05-21 9:44 AM, Weiming Shi wrote:
> On 26-05-20 20:58, Willem de Bruijn wrote:
>> Dongli Zhang wrote:
>>>
>>>
>>> On 2026-05-20 5:05 PM, Willem de Bruijn wrote:
>>>> Weiming Shi wrote:
>>>>> tun_xdp_one() returns -EINVAL on a frame shorter than ETH_HLEN without
>>>>> freeing the page that vhost_net_build_xdp() allocated for it.
>>>>> tun_sendmsg() discards that -EINVAL and still returns total_len, so
>>>>> vhost_tx_batch() takes the success path and never frees the page; each
>>>>> short frame in a batch leaks one page-frag chunk.
>>>>>
>>>>> A local process that can open /dev/net/tun and /dev/vhost-net can hit
>>>>> this path: it attaches a tun/tap device as the vhost-net backend and
>>>>> feeds TX descriptors whose length minus the virtio-net header is below
>>>>> ETH_HLEN. Each kick leaks the page-frag chunks for that batch, and a
>>>>> tight submission loop exhausts host memory and triggers an OOM panic.
>>>>> Free the page before returning -EINVAL, matching the XDP-program error
>>>>> path in the same function.
>>>>>
>>>>> Fixes: 049584807f1d ("tun: add missing verification for short frame")
>>>>> Reported-by: Xiang Mei <xmei5@asu.edu>
>>>>> Assisted-by: Claude:claude-opus-4-7
>>>>> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
>>>>> ---
>>>>>  drivers/net/tun.c | 4 +++-
>>>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>>>>> index b183189f1853..f594360d66d6 100644
>>>>> --- a/drivers/net/tun.c
>>>>> +++ b/drivers/net/tun.c
>>>>> @@ -2394,8 +2394,10 @@ static int tun_xdp_one(struct tun_struct *tun,
>>>>>  	bool skb_xdp = false;
>>>>>  	struct page *page;
>>>>>  
>>>>> -	if (unlikely(datasize < ETH_HLEN))
>>>>> +	if (unlikely(datasize < ETH_HLEN)) {
>>>>> +		put_page(virt_to_head_page(xdp->data));
>>>>>  		return -EINVAL;
>>>>> +	}
>>>>
>>>> Make sense, thanks.
>>>>
>>>> The error path from tun_xdp_act does the same. And the default: label
>>>> used to too, before a batching optimization was introduced.
>>>>
>>>> Is the same then also missing if build_skb fails?
>>>>
>>>
>>> I also agree that we may need to handle build_skb() failure.
>>
>> Thanks. Fine to defer to a separate patch btw. Either way.
>>  
>>> In addition, I think we may need this fix for tap_get_user_xdp() as well.
>>>
>>> Thank you very much!
>>>
>>> Dongli Zhang
>>
>>
> Hi,
> 
> I have sent a patch to fix the same issue  and the patch is under review now. 
> Please take a look if you have time, thanks!
> 
> https://urldefense.com/v3/__https://lore.kernel.org/all/20260521163230.1478627-2-bestswngs@gmail.com/__;!!ACWV5N9M2RV99hQ!IDJKNE6zGJJ7hjjCFU8OviGWfh4A_mNZAhrTowR0Kb4p5bm4JmoCmEI6_GfOXFjcxd2ctJ3pbcJypErFgC4-$ 
> https://urldefense.com/v3/__https://lore.kernel.org/all/20260521163312.1479805-2-bestswngs@gmail.com/__;!!ACWV5N9M2RV99hQ!IDJKNE6zGJJ7hjjCFU8OviGWfh4A_mNZAhrTowR0Kb4p5bm4JmoCmEI6_GfOXFjcxd2ctJ3pbcJypMyOq4Op$ 
> 

The bug fix applies to both TAP and TUN.

TAP has a single patch, while TUN has two separate patches. I'm not sure which
approach is better.

[PATCH net] tap: free page on error paths in tap_get_user_xdp()
https://lore.kernel.org/netdev/20260521163230.1478627-2-bestswngs@gmail.com

[PATCH net] tun: free page on short-frame rejection in tun_xdp_one()
https://lore.kernel.org/all/20260520160020.375349-2-bestswngs@gmail.com

[PATCH net] tun: free page on build_skb failure in tun_xdp_one()
https://lore.kernel.org/netdev/20260521163312.1479805-2-bestswngs@gmail.com/


The bug fix looks good to me.

Reviewed-by: Dongli Zhang <dongli.zhang@oracle.com>

+ Michael for FYI regarding vhost as well.

Thank you very much!

Dongli Zhang

  reply	other threads:[~2026-05-21 21:01 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-20 16:00 Weiming Shi
2026-05-21  0:05 ` Willem de Bruijn
2026-05-21  0:37   ` Dongli Zhang
2026-05-21  0:58     ` Willem de Bruijn
2026-05-21 16:44       ` Weiming Shi
2026-05-21 21:01         ` Dongli Zhang [this message]
2026-05-22 13:39 ` Willem de Bruijn
2026-05-22 15:00 ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cc1ea6d3-f694-481c-a5db-4356c5076207@oracle.com \
    --to=dongli.zhang@oracle.com \
    --cc=bestswngs@gmail.com \
    --cc=edumazet@google.com \
    --cc=jasowang@redhat.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=si-wei.liu@oracle.com \
    --cc=willemdebruijn.kernel@gmail.com \
    --cc=xmei5@asu.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®