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
>
> I am worried that the skb might be inserted into another skb list before
> skb_dequeue is called, but I can’t find such code, it seems this worry
> is unnecessary.
