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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAKhg4tJen5JQp-cpvdmdzy1RYJL_=a0bk6TYs0ud0G1rn1ebsg@mail.gmail.com>
Date: Thu, 3 Aug 2023 16:25:49 +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 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.

Thanks,
Liang

> > moment(a little nagetive impact on skb_try_coalesce), to avoid potential
>
>                   ^^^^^^^^
>                   negative
>

Sure, Thanks.


> > issues associated with using incorrect skb->pp_recycle information,
> > setting skb->pp_recycle to 0 to reflect the pp state of the skb.
> >
> > Signed-off-by: Liang Chen <liangchen.linux@...il.com>
>
> I won't say for sure, but may be a candidate for the fixes tree, not
> next. This way it would need a "Fixes:" tag here (above the SoB).
>
> > ---
> >  net/core/dev.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 10e5a036c706..07baf72be7d7 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -4934,6 +4934,8 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
> >                       goto do_drop;
> >               if (skb_linearize(skb))
> >                       goto do_drop;
> > +             if (skb->pp_recycle)
> > +                     skb->pp_recycle = 0;
> >       }
> >
> >       act = bpf_prog_run_generic_xdp(skb, xdp, xdp_prog);
>
> Thanks,
> Olek