netdev - Re: [PATCH net-next] xdp: Fixing skb->pp_recycle flag in generic XDP handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5f9f6107-3617-dca7-0551-51fc54861298@intel.com>
Date: Thu, 3 Aug 2023 17:14:36 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Liang Chen <liangchen.linux@...il.com>
CC: <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
	<pabeni@...hat.com>, <ast@...nel.org>, <daniel@...earbox.net>,
	<john.fastabend@...il.com>, <ilias.apalodimas@...aro.org>,
	<netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] xdp: Fixing skb->pp_recycle flag in generic XDP
 handling

From: Liang Chen <liangchen.linux@...il.com>
Date: Thu, 3 Aug 2023 16:25:49 +0800

> On Thu, Aug 3, 2023 at 1:11 AM Alexander Lobakin
> <aleksander.lobakin@...el.com> wrote:
>>
>> From: Liang Chen <liangchen.linux@...il.com>
>> Date: Wed,  2 Aug 2023 15:04:54 +0800
>>
>>> In the generic XDP processing flow, if an skb with a page pool page
>>> (skb->pp_recycle == 1) fails to meet XDP packet requirements, it will
>>> undergo head expansion and linearization of fragment data. As a result,
>>> skb->head points to a reallocated buffer without any fragments. At this
>>> point, the skb will not contain any page pool pages. However, the
>>> skb->pp_recycle flag is still set to 1, which is inconsistent with the
>>> actual situation. Although it doesn't seem to cause much real harm at the
>>
>> This means it must be handled in the function which replaces the head,
>> i.e. pskb_expand_head(). Your change only suppresses one symptom of the
>> issue.
>>
> 
> I attempted to do so. But after pskb_expand_head, there may still be
> skb frags with pp pages left. It is after skb_linearize those frags
> are removed.

Ah, right.
Then you need to handle that in __pskb_pull_tail(). Check at the end of
the function whether the skb still has any frags, and if not, clear
skb->pp_recycle.

The most correct fix would be to do that in both pskb_expand_head() and
__pskb_pull_tail(): iterate over the frags and check if any page still
belongs to a page_pool. Then page_pool_return_skb_page() wouldn't hit
false-branch after the skb was re-layout.

[...]

Thanks,
Olek