[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <64f2480035813_346b0208f5@john.notmuch>
Date: Fri, 01 Sep 2023 13:22:24 -0700
From: John Fastabend <john.fastabend@...il.com>
To: Jiri Olsa <olsajiri@...il.com>,
Xu Kuohai <xukuohai@...wei.com>
Cc: John Fastabend <john.fastabend@...il.com>,
Jiri Olsa <olsajiri@...il.com>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>,
netdev@...r.kernel.org,
bpf@...r.kernel.org,
Martin KaFai Lau <kafai@...com>,
Song Liu <songliubraving@...com>,
Yonghong Song <yhs@...com>,
KP Singh <kpsingh@...omium.org>,
Stanislav Fomichev <sdf@...gle.com>,
Hao Luo <haoluo@...gle.com>,
Hou Tao <houtao1@...wei.com>,
Bobby Eshleman <bobby.eshleman@...edance.com>
Subject: Re: [BUG bpf-next] bpf/net: Hitting gpf when running selftests
Jiri Olsa wrote:
> On Fri, Sep 01, 2023 at 05:10:43PM +0800, Xu Kuohai wrote:
>
> SNIP
>
> > > > Trying to come up with some nice fix now.
> > >
> > > Something like this it fixes the splat, but need to think if it
> > > introduces anything or some better way to do this. Basic idea
> > > is to bump user->refcnt because we have two references to the
> > > skb and want to ensure we really only kfree_skb() the skb
> > > after both references are dropped.
> > >
> > > diff --git a/net/core/skmsg.c b/net/core/skmsg.c
> > > index a0659fc29bcc..6c31eefbd777 100644
> > > --- a/net/core/skmsg.c
> > > +++ b/net/core/skmsg.c
> > > @@ -612,12 +612,18 @@ static int sk_psock_skb_ingress_self(struct sk_psock *psock, struct sk_buff *skb
> > > static int sk_psock_handle_skb(struct sk_psock *psock, struct sk_buff *skb,
> > > u32 off, u32 len, bool ingress)
> > > {
> > > + int err = 0;
> > > +
> > > if (!ingress) {
> > > if (!sock_writeable(psock->sk))
> > > return -EAGAIN;
> > > return skb_send_sock(psock->sk, skb, off, len);
> > > }
> > > - return sk_psock_skb_ingress(psock, skb, off, len);
> > > + skb_get(skb);
> > > + err = sk_psock_skb_ingress(psock, skb, off, len);
> > > + if (err < 0)
> > > + kfree_skb(skb);
> > > + return err;
> > > }
> > > static void sk_psock_skb_state(struct sk_psock *psock,
> > > @@ -685,9 +691,7 @@ static void sk_psock_backlog(struct work_struct *work)
> > > } while (len);
> > > skb = skb_dequeue(&psock->ingress_skb);
> > > - if (!ingress) {
> > > - kfree_skb(skb);
> > > - }
> > > + kfree_skb(skb);
> > > }
> > > end:
> > > mutex_unlock(&psock->work_mutex);
> > > .
> >
> > With this fix, the crash is gone.
>
> +1, same on my setup
>
> jirka
Sent a patch. Add tested-by and acks if you have time. Thanks!
Powered by blists - more mailing lists