On Mon, Jan 13, 2020 at 11:42:42PM +0100, Jakub Sitnicki wrote:
> On Mon, Jan 13, 2020 at 11:23 PM CET, Martin Lau wrote:
> > On Fri, Jan 10, 2020 at 11:50:20AM +0100, Jakub Sitnicki wrote:
> >> Prepare for cloning listening sockets that have their protocol callbacks
> >> overridden by sk_msg. Child sockets must not inherit parent callbacks that
> >> access state stored in sk_user_data owned by the parent.
> >>
> >> Restore the child socket protocol callbacks before the it gets hashed and
> >> any of the callbacks can get invoked.
> >>
> >> Signed-off-by: Jakub Sitnicki <jakub@xxxxxxxxxxxxxx>
> >> ---
> >> include/net/tcp.h | 1 +
> >> net/ipv4/tcp_bpf.c | 13 +++++++++++++
> >> net/ipv4/tcp_minisocks.c | 2 ++
> >> 3 files changed, 16 insertions(+)
> >>
> >> diff --git a/include/net/tcp.h b/include/net/tcp.h
> >> index 9dd975be7fdf..7cbf9465bb10 100644
> >> --- a/include/net/tcp.h
> >> +++ b/include/net/tcp.h
> >> @@ -2181,6 +2181,7 @@ int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
> >> int nonblock, int flags, int *addr_len);
> >> int __tcp_bpf_recvmsg(struct sock *sk, struct sk_psock *psock,
> >> struct msghdr *msg, int len, int flags);
> >> +void tcp_bpf_clone(const struct sock *sk, struct sock *child);
> >>
> >> /* Call BPF_SOCK_OPS program that returns an int. If the return value
> >> * is < 0, then the BPF op failed (for example if the loaded BPF
> >> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> >> index f6c83747c71e..6f96320fb7cf 100644
> >> --- a/net/ipv4/tcp_bpf.c
> >> +++ b/net/ipv4/tcp_bpf.c
> >> @@ -586,6 +586,19 @@ static void tcp_bpf_close(struct sock *sk, long timeout)
> >> saved_close(sk, timeout);
> >> }
> >>
> >> +/* If a child got cloned from a listening socket that had tcp_bpf
> >> + * protocol callbacks installed, we need to restore the callbacks to
> >> + * the default ones because the child does not inherit the psock state
> >> + * that tcp_bpf callbacks expect.
> >> + */
> >> +void tcp_bpf_clone(const struct sock *sk, struct sock *newsk)
> >> +{
> >> + struct proto *prot = newsk->sk_prot;
> >> +
> >> + if (prot->recvmsg == tcp_bpf_recvmsg)
> > A question not related to this patch (may be it is more for patch 6).
> >
> > How tcp_bpf_recvmsg may be used for a listening sock (sk here)?
>
> It can't be used. It's a way of checking if sock has tcp_bpf callbacks
> that I copied from sk_psock_get_checked:
ic. It seems only tcp_bpf_close and tcp_bpf_unhash may be useful.
Asking because it intuitively made me think how tcp_bpf_recvmsg/sendmsg/...etc
may be used since they are also set to listening sk.
>
> static inline struct sk_psock *sk_psock_get_checked(struct sock *sk)
> {
> struct sk_psock *psock;
>
> rcu_read_lock();
> psock = sk_psock(sk);
> if (psock) {
> if (sk->sk_prot->recvmsg != tcp_bpf_recvmsg) {
> psock = ERR_PTR(-EBUSY);
> goto out;
> }
> ...
>
> This makes me think that perhaps it deserves a well-named helper.
>
> >
> >> + newsk->sk_prot = sk->sk_prot_creator;
> >> +}
> >> +
