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
| ||
|
Message-ID: <CAKhg4t+SRCXMiuJGMYNw+=xDMW84NtpmKuGw=4L+dkTqX7E7oQ@mail.gmail.com> Date: Mon, 7 Aug 2023 20:49:34 +0800 From: Liang Chen <liangchen.linux@...il.com> To: Alexander Lobakin <aleksander.lobakin@...el.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 On Thu, Aug 3, 2023 at 11:17 PM Alexander Lobakin <aleksander.lobakin@...el.com> wrote: > > 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. > Yeah, I agree. netif_receive_generic_xdp may not be the only place the flag can get wrong. To make the pp_recycle flag strictly reflect the state of page pool usages, the check should be put in both functions. But, since it's merely an indication that the skb is 'page pool aware,' and considering that the performance impact we observed doesn't affect the native XDP path, addressing this issue doesn't seem worthwhile based on the feedback I've received. Thanks, Liang > [...] > > Thanks, > Olek
Powered by blists - more mailing lists