Cong Wang wrote:
> On Mon, Apr 5, 2021 at 1:25 AM Eric Dumazet <eric.dumazet@xxxxxxxxx> wrote:
> >
> >
> >
> > On 3/31/21 4:32 AM, Cong Wang wrote:
> > > From: Cong Wang <cong.wang@xxxxxxxxxxxxx>
> > >
> > > Currently sockmap calls into each protocol to update the struct
> > > proto and replace it. This certainly won't work when the protocol
> > > is implemented as a module, for example, AF_UNIX.
> > >
> > > Introduce a new ops sk->sk_prot->psock_update_sk_prot(), so each
> > > protocol can implement its own way to replace the struct proto.
> > > This also helps get rid of symbol dependencies on CONFIG_INET.
> >
> > [...]
> >
> >
> > >
> > > -struct proto *tcp_bpf_get_proto(struct sock *sk, struct sk_psock *psock)
> > > +int tcp_bpf_update_proto(struct sock *sk, bool restore)
> > > {
> > > + struct sk_psock *psock = sk_psock(sk);
> >
> > I do not think RCU is held here ?
> >
> > sk_psock() is using rcu_dereference_sk_user_data()
>
> Right, I just saw the syzbot report. But here we already have
> the writer lock of sk_callback_lock, hence RCU read lock here
> makes no sense to me. Probably we just have to tell RCU we
> already have sk_callback_lock.
>
> Thanks.
I think you need to ensure its the psock we originally grabbed as
well. Otherwise how do we ensure the psock is not swapped from
another thread?
