On Mon, Aug 2, 2021 at 2:19 PM Jiang Wang jiang.wang@bytedance.com wrote:

Previously, sockmap for AF_UNIX protocol only supports dgram type. This patch add unix stream type support, which is similar to unix_dgram_proto. To support sockmap, dgram and stream cannot share the same unix_proto anymore, because they have different implementations, such as unhash for stream type (which will remove closed or disconnected sockets from the map), so rename unix_proto to unix_dgram_proto and add a new unix_stream_proto.

Also implement stream related sockmap functions. And add dgram key words to those dgram specific functions.

Signed-off-by: Jiang Wang jiang.wang@bytedance.com Reviewed-by: Cong Wang cong.wang@bytedance.com

---

include/net/af_unix.h | 8 +++- net/core/sock_map.c | 8 +++- net/unix/af_unix.c | 74 ++++++++++++++++++++++++++++----- net/unix/unix_bpf.c | 96 +++++++++++++++++++++++++++++++++---------- 4 files changed, 150 insertions(+), 36 deletions(-)

diff --git a/include/net/af_unix.h b/include/net/af_unix.h index 435a2c3d5..5d04fbf8a 100644 --- a/include/net/af_unix.h +++ b/include/net/af_unix.h @@ -84,6 +84,8 @@ long unix_outq_len(struct sock *sk);

int __unix_dgram_recvmsg(struct sock *sk, struct msghdr *msg, size_t size, int flags); +int __unix_stream_recvmsg(struct sock *sk, struct msghdr *msg, size_t size,

•                     int flags);
  

#ifdef CONFIG_SYSCTL int unix_sysctl_register(struct net *net); void unix_sysctl_unregister(struct net *net); @@ -93,9 +95,11 @@ static inline void unix_sysctl_unregister(struct net *net) {} #endif

#ifdef CONFIG_BPF_SYSCALL -extern struct proto unix_proto; +extern struct proto unix_dgram_proto; +extern struct proto unix_stream_proto;

-int unix_bpf_update_proto(struct sock *sk, struct sk_psock *psock, bool restore); +int unix_dgram_bpf_update_proto(struct sock *sk, struct sk_psock *psock, bool restore); +int unix_stream_bpf_update_proto(struct sock *sk, struct sk_psock *psock, bool restore); void __init unix_bpf_build_proto(void); #else static inline void __init unix_bpf_build_proto(void) diff --git a/net/core/sock_map.c b/net/core/sock_map.c index ae5fa4338..42f50ea7a 100644 --- a/net/core/sock_map.c +++ b/net/core/sock_map.c @@ -517,9 +517,15 @@ static bool sk_is_tcp(const struct sock *sk) sk->sk_protocol == IPPROTO_TCP; }

+static bool sk_is_unix_stream(const struct sock *sk) +{

•   return sk->sk_type == SOCK_STREAM &&
  

•          sk->sk_protocol == PF_UNIX;
  

+}

static bool sock_map_redirect_allowed(const struct sock *sk) {

•   if (sk_is_tcp(sk))
  

•   if (sk_is_tcp(sk) || sk_is_unix_stream(sk))
            return sk->sk_state != TCP_LISTEN;
    else
            return sk->sk_state == TCP_ESTABLISHED;
  

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c index 0ae3fc4c8..9c1711c67 100644 --- a/net/unix/af_unix.c +++ b/net/unix/af_unix.c @@ -791,17 +791,35 @@ static void unix_close(struct sock *sk, long timeout) */ }

-struct proto unix_proto = {

•   .name                   = "UNIX",
  

+static void unix_unhash(struct sock *sk) +{

•   /* Nothing to do here, unix socket does not need a ->unhash().
  

•    * This is merely for sockmap.
  

•    */
  

+}

+struct proto unix_dgram_proto = {

•   .name                   = "UNIX-DGRAM",
  

•   .owner                  = THIS_MODULE,
  

•   .obj_size               = sizeof(struct unix_sock),
  

•   .close                  = unix_close,
  

+#ifdef CONFIG_BPF_SYSCALL

•   .psock_update_sk_prot   = unix_dgram_bpf_update_proto,
  

+#endif +};

+struct proto unix_stream_proto = {

•   .name                   = "UNIX-STREAM",
    .owner                  = THIS_MODULE,
    .obj_size               = sizeof(struct unix_sock),
    .close                  = unix_close,
  

•   .unhash                 = unix_unhash,
  

#ifdef CONFIG_BPF_SYSCALL

•   .psock_update_sk_prot   = unix_bpf_update_proto,
  

•   .psock_update_sk_prot   = unix_stream_bpf_update_proto,
  

#endif };

-static struct sock *unix_create1(struct net *net, struct socket *sock, int kern) +static struct sock *unix_create1(struct net *net, struct socket *sock, int kern, int type) { struct sock *sk = NULL; struct unix_sock *u; @@ -810,7 +828,11 @@ static struct sock *unix_create1(struct net *net, struct socket *sock, int kern) if (atomic_long_read(&unix_nr_socks) > 2 * get_max_files()) goto out;

•   sk = sk_alloc(net, PF_UNIX, GFP_KERNEL, &unix_proto, kern);
  

•   if (type == SOCK_STREAM)
  

•           sk = sk_alloc(net, PF_UNIX, GFP_KERNEL, &unix_stream_proto, kern);
  

•   else /*dgram and  seqpacket */
  

•           sk = sk_alloc(net, PF_UNIX, GFP_KERNEL, &unix_dgram_proto, kern);
  

•   if (!sk)
            goto out;
  
  

@@ -872,7 +894,7 @@ static int unix_create(struct net *net, struct socket *sock, int protocol, return -ESOCKTNOSUPPORT; }

•   return unix_create1(net, sock, kern) ? 0 : -ENOMEM;
  

•   return unix_create1(net, sock, kern, sock->type) ? 0 : -ENOMEM;
  

}

static int unix_release(struct socket *sock) @@ -1286,7 +1308,7 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr *uaddr, err = -ENOMEM;

    /* create new sock for complete connection */

•   newsk = unix_create1(sock_net(sk), NULL, 0);
  

•   newsk = unix_create1(sock_net(sk), NULL, 0, sock->type);
    if (newsk == NULL)
            goto out;
  
  

@@ -2214,7 +2236,7 @@ static int unix_dgram_recvmsg(struct socket *sock, struct msghdr *msg, size_t si struct sock *sk = sock->sk;

#ifdef CONFIG_BPF_SYSCALL

•   if (sk->sk_prot != &unix_proto)
  

•   if (sk->sk_prot != &unix_dgram_proto)
            return sk->sk_prot->recvmsg(sk, msg, size, flags & MSG_DONTWAIT,
                                        flags & ~MSG_DONTWAIT, NULL);
  

#endif @@ -2533,6 +2555,20 @@ static int unix_stream_read_actor(struct sk_buff *skb, return ret ?: chunk; }

+int __unix_stream_recvmsg(struct sock *sk, struct msghdr *msg,

•                     size_t size, int flags)
  

+{

•   struct unix_stream_read_state state = {
  

•           .recv_actor = unix_stream_read_actor,
  

•           .socket = sk->sk_socket,
  

•           .msg = msg,
  

•           .size = size,
  

•           .flags = flags
  

•   };
  

•   return unix_stream_read_generic(&state, true);
  

+}

static int unix_stream_recvmsg(struct socket *sock, struct msghdr *msg, size_t size, int flags) { @@ -2544,6 +2580,12 @@ static int unix_stream_recvmsg(struct socket *sock, struct msghdr *msg, .flags = flags };

+#ifdef CONFIG_BPF_SYSCALL

•   struct sock *sk = sock->sk;
  

•   if (sk->sk_prot != &unix_stream_proto)
  

•           return sk->sk_prot->recvmsg(sk, msg, size, flags & MSG_DONTWAIT,
  

•                                       flags & ~MSG_DONTWAIT, NULL);
  

+#endif return unix_stream_read_generic(&state, true); }

@@ -2605,6 +2647,7 @@ static int unix_shutdown(struct socket *sock, int mode)

            int peer_mode = 0;

•           other->sk_prot->unhash(other);
            if (mode&RCV_SHUTDOWN)
                    peer_mode |= SEND_SHUTDOWN;
            if (mode&SEND_SHUTDOWN)
  

@@ -2613,8 +2656,10 @@ static int unix_shutdown(struct socket *sock, int mode) other->sk_shutdown |= peer_mode; unix_state_unlock(other); other->sk_state_change(other);

•           if (peer_mode == SHUTDOWN_MASK)
  

•           if (peer_mode == SHUTDOWN_MASK) {
                    sk_wake_async(other, SOCK_WAKE_WAITD, POLL_HUP);
  

•                   other->sk_state = TCP_CLOSE;
  

•           }
            else if (peer_mode & RCV_SHUTDOWN)
                    sk_wake_async(other, SOCK_WAKE_WAITD, POLL_IN);
    }
  

@@ -2993,7 +3038,13 @@ static int __init af_unix_init(void)

    BUILD_BUG_ON(sizeof(struct unix_skb_parms) > sizeof_field(struct sk_buff, cb));

•   rc = proto_register(&unix_proto, 1);
  

•   rc = proto_register(&unix_dgram_proto, 1);
  

•   if (rc != 0) {
  

•           pr_crit("%s: Cannot create unix_sock SLAB cache!\n", __func__);
  

•           goto out;
  

•   }
  

•   rc = proto_register(&unix_stream_proto, 1);
    if (rc != 0) {
            pr_crit("%s: Cannot create unix_sock SLAB cache!\n", __func__);
            goto out;
  

@@ -3009,7 +3060,8 @@ static int __init af_unix_init(void) static void __exit af_unix_exit(void) { sock_unregister(PF_UNIX);

•   proto_unregister(&unix_proto);
  

•   proto_unregister(&unix_dgram_proto);
  

•   proto_unregister(&unix_stream_proto);
    unregister_pernet_subsys(&unix_net_ops);
  

}

diff --git a/net/unix/unix_bpf.c b/net/unix/unix_bpf.c index db0cda29f..17e210666 100644 --- a/net/unix/unix_bpf.c +++ b/net/unix/unix_bpf.c @@ -38,9 +38,18 @@ static int unix_msg_wait_data(struct sock *sk, struct sk_psock *psock, return ret; }

-static int unix_dgram_bpf_recvmsg(struct sock *sk, struct msghdr *msg,

•                             size_t len, int nonblock, int flags,
  

•                             int *addr_len)
  

+static int __unix_recvmsg(struct sock *sk, struct msghdr *msg,

•                      size_t len, int flags)
  

+{

•   if (sk->sk_type == SOCK_DGRAM)
  

•           return __unix_dgram_recvmsg(sk, msg, len, flags);
  

•   else
  

•           return __unix_stream_recvmsg(sk, msg, len, flags);
  

+}

+static int unix_bpf_recvmsg(struct sock *sk, struct msghdr *msg,

•                       size_t len, int nonblock, int flags,
  

•                       int *addr_len)
  

{ struct unix_sock *u = unix_sk(sk); struct sk_psock *psock; @@ -48,12 +57,12 @@ static int unix_dgram_bpf_recvmsg(struct sock *sk, struct msghdr *msg,

    psock = sk_psock_get(sk);
    if (unlikely(!psock))

•           return __unix_dgram_recvmsg(sk, msg, len, flags);
  

•           return __unix_recvmsg(sk, msg, len, flags);
  
    mutex_lock(&u->iolock);
    if (!skb_queue_empty(&sk->sk_receive_queue) &&
        sk_psock_queue_empty(psock)) {
  

•           ret = __unix_dgram_recvmsg(sk, msg, len, flags);
  

•           ret = __unix_recvmsg(sk, msg, len, flags);
            goto out;
    }
  
  

@@ -68,7 +77,7 @@ static int unix_dgram_bpf_recvmsg(struct sock *sk, struct msghdr *msg, if (data) { if (!sk_psock_queue_empty(psock)) goto msg_bytes_ready;

•                   ret = __unix_dgram_recvmsg(sk, msg, len, flags);
  

•                   ret = __unix_recvmsg(sk, msg, len, flags);
                    goto out;
            }
            copied = -EAGAIN;
  

@@ -80,43 +89,86 @@ static int unix_dgram_bpf_recvmsg(struct sock *sk, struct msghdr *msg, return ret; }

-static struct proto *unix_prot_saved __read_mostly; -static DEFINE_SPINLOCK(unix_prot_lock); -static struct proto unix_bpf_prot; +static struct proto *unix_dgram_prot_saved __read_mostly; +static DEFINE_SPINLOCK(unix_dgram_prot_lock); +static struct proto unix_dgram_bpf_prot;

+static struct proto *unix_stream_prot_saved __read_mostly; +static DEFINE_SPINLOCK(unix_stream_prot_lock); +static struct proto unix_stream_bpf_prot;

+static void unix_dgram_bpf_rebuild_protos(struct proto *prot, const struct proto *base) +{

•   *prot        = *base;
  

•   prot->close  = sock_map_close;
  

•   prot->recvmsg = unix_bpf_recvmsg;
  

+}

-static void unix_bpf_rebuild_protos(struct proto *prot, const struct proto *base) +static void unix_stream_bpf_rebuild_protos(struct proto *prot,

•                                      const struct proto *base)
  

{ *prot = *base; prot->close = sock_map_close;

•   prot->recvmsg = unix_dgram_bpf_recvmsg;
  

•   prot->recvmsg = unix_bpf_recvmsg;
  

•   prot->unhash  = sock_map_unhash;
  

+}

+static void unix_dgram_bpf_check_needs_rebuild(struct proto *ops) +{

•   if (unlikely(ops != smp_load_acquire(&unix_dgram_prot_saved))) {
  

•           spin_lock_bh(&unix_dgram_prot_lock);
  

•           if (likely(ops != unix_dgram_prot_saved)) {
  

•                   unix_dgram_bpf_rebuild_protos(&unix_dgram_bpf_prot, ops);
  

•                   smp_store_release(&unix_dgram_prot_saved, ops);
  

•           }
  

•           spin_unlock_bh(&unix_dgram_prot_lock);
  

•   }
  

}

-static void unix_bpf_check_needs_rebuild(struct proto *ops) +static void unix_stream_bpf_check_needs_rebuild(struct proto *ops) {

•   if (unlikely(ops != smp_load_acquire(&unix_prot_saved))) {
  

•           spin_lock_bh(&unix_prot_lock);
  

•           if (likely(ops != unix_prot_saved)) {
  

•                   unix_bpf_rebuild_protos(&unix_bpf_prot, ops);
  

•                   smp_store_release(&unix_prot_saved, ops);
  

•   if (unlikely(ops != smp_load_acquire(&unix_stream_prot_saved))) {
  

•           spin_lock_bh(&unix_stream_prot_lock);
  

•           if (likely(ops != unix_stream_prot_saved)) {
  

•                   unix_stream_bpf_rebuild_protos(&unix_stream_bpf_prot, ops);
  

•                   smp_store_release(&unix_stream_prot_saved, ops);
            }
  

•           spin_unlock_bh(&unix_prot_lock);
  

•           spin_unlock_bh(&unix_stream_prot_lock);
  

•   }
  

+}

+int unix_dgram_bpf_update_proto(struct sock *sk, struct sk_psock *psock, bool restore) +{

•   if (restore) {
  

•           sk->sk_write_space = psock->saved_write_space;
  

•           WRITE_ONCE(sk->sk_prot, psock->sk_proto);
  

•           return 0;
    }
  

•   unix_dgram_bpf_check_needs_rebuild(psock->sk_proto);
  

•   WRITE_ONCE(sk->sk_prot, &unix_dgram_bpf_prot);
  

•   return 0;
  

}

-int unix_bpf_update_proto(struct sock *sk, struct sk_psock *psock, bool restore) +int unix_stream_bpf_update_proto(struct sock *sk, struct sk_psock *psock, bool restore) {

•   if (sk->sk_type != SOCK_STREAM)
  

•           return -EOPNOTSUPP;
  

Cong reminded me that there is no need to check sk_type here because unix_stream_bpf_prot is only used by stream type. I will remove this part in the next version.

    if (restore) {
            sk->sk_write_space = psock->saved_write_space;
            WRITE_ONCE(sk->sk_prot, psock->sk_proto);
            return 0;
    }

•   unix_bpf_check_needs_rebuild(psock->sk_proto);
  

•   WRITE_ONCE(sk->sk_prot, &unix_bpf_prot);
  

•   unix_stream_bpf_check_needs_rebuild(psock->sk_proto);
  

•   WRITE_ONCE(sk->sk_prot, &unix_stream_bpf_prot);
    return 0;
  

}

void __init unix_bpf_build_proto(void) {

•   unix_bpf_rebuild_protos(&unix_bpf_prot, &unix_proto);
  

•   unix_dgram_bpf_rebuild_protos(&unix_dgram_bpf_prot, &unix_dgram_proto);
  

•   unix_stream_bpf_rebuild_protos(&unix_stream_bpf_prot, &unix_stream_proto);
  

}

2.20.1

On Wed, Aug 4, 2021 at 9:59 AM Jakub Sitnicki jakub@cloudflare.com wrote:

On Mon, Aug 02, 2021 at 11:19 PM CEST, Jiang Wang wrote:

[...]

...

diff --git a/net/core/sock_map.c b/net/core/sock_map.c index ae5fa4338..42f50ea7a 100644 --- a/net/core/sock_map.c +++ b/net/core/sock_map.c @@ -517,9 +517,15 @@ static bool sk_is_tcp(const struct sock *sk) sk->sk_protocol == IPPROTO_TCP; }

+static bool sk_is_unix_stream(const struct sock *sk) +{

• return sk->sk_type == SOCK_STREAM &&
  

•        sk->sk_protocol == PF_UNIX;
  

+}

static bool sock_map_redirect_allowed(const struct sock *sk) {

• if (sk_is_tcp(sk))
  

• if (sk_is_tcp(sk) || sk_is_unix_stream(sk))
          return sk->sk_state != TCP_LISTEN;
  else
          return sk->sk_state == TCP_ESTABLISHED;
  

Let me provide some context.

The reason why we check != TCP_LISTEN for TCP sockets is that we want to allow redirect redirect to sockets that are about to transition from TCP_SYN_RECV to TCP_ESTABLISHED, in addition to sockets already in TCP_ESTABLISHED state.

That's because BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB callback happens when socket is still in TCP_SYN_RECV state. With BPF sockops program, we can insert such socket into a sockmap. Hence, there is a short window of opportunity when we could redirect to a socket in TCP_SYN_RECV.

UNIX sockets can be only in TCP_{CLOSE,LISTEN,ESTABLISHED} state, AFAIK. So it is sufficient to rely on the default == TCP_ESTABLISHED check.

Got it. Thanks for the explanation. I will change unix sockets to only check == TCP_ESTABLISHED condition.

...

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c index 0ae3fc4c8..9c1711c67 100644 --- a/net/unix/af_unix.c +++ b/net/unix/af_unix.c @@ -791,17 +791,35 @@ static void unix_close(struct sock *sk, long timeout) */ }

-struct proto unix_proto = {

• .name                   = "UNIX",
  

+static void unix_unhash(struct sock *sk) +{

• /* Nothing to do here, unix socket does not need a ->unhash().
  

•  * This is merely for sockmap.
  

•  */
  

+}

+struct proto unix_dgram_proto = {

• .name                   = "UNIX-DGRAM",
  

• .owner                  = THIS_MODULE,
  

• .obj_size               = sizeof(struct unix_sock),
  

• .close                  = unix_close,
  

+#ifdef CONFIG_BPF_SYSCALL

• .psock_update_sk_prot   = unix_dgram_bpf_update_proto,
  

+#endif +};

+struct proto unix_stream_proto = {

• .name                   = "UNIX-STREAM",
  .owner                  = THIS_MODULE,
  .obj_size               = sizeof(struct unix_sock),
  .close                  = unix_close,
  

• .unhash                 = unix_unhash,
  

#ifdef CONFIG_BPF_SYSCALL

• .psock_update_sk_prot   = unix_bpf_update_proto,
  

• .psock_update_sk_prot   = unix_stream_bpf_update_proto,
  

#endif };

-static struct sock *unix_create1(struct net *net, struct socket *sock, int kern) +static struct sock *unix_create1(struct net *net, struct socket *sock, int kern, int type) { struct sock *sk = NULL; struct unix_sock *u; @@ -810,7 +828,11 @@ static struct sock *unix_create1(struct net *net, struct socket *sock, int kern) if (atomic_long_read(&unix_nr_socks) > 2 * get_max_files()) goto out;

• sk = sk_alloc(net, PF_UNIX, GFP_KERNEL, &unix_proto, kern);
  

• if (type == SOCK_STREAM)
  

•         sk = sk_alloc(net, PF_UNIX, GFP_KERNEL, &unix_stream_proto, kern);
  

• else /*dgram and  seqpacket */
  

•         sk = sk_alloc(net, PF_UNIX, GFP_KERNEL, &unix_dgram_proto, kern);
  

• if (!sk)
          goto out;
  
  

@@ -872,7 +894,7 @@ static int unix_create(struct net *net, struct socket *sock, int protocol, return -ESOCKTNOSUPPORT; }

• return unix_create1(net, sock, kern) ? 0 : -ENOMEM;
  

• return unix_create1(net, sock, kern, sock->type) ? 0 : -ENOMEM;
  

}

static int unix_release(struct socket *sock) @@ -1286,7 +1308,7 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr *uaddr, err = -ENOMEM;

  /* create new sock for complete connection */

• newsk = unix_create1(sock_net(sk), NULL, 0);
  

• newsk = unix_create1(sock_net(sk), NULL, 0, sock->type);
  if (newsk == NULL)
          goto out;
  
  

@@ -2214,7 +2236,7 @@ static int unix_dgram_recvmsg(struct socket *sock, struct msghdr *msg, size_t si struct sock *sk = sock->sk;

#ifdef CONFIG_BPF_SYSCALL

• if (sk->sk_prot != &unix_proto)
  

• if (sk->sk_prot != &unix_dgram_proto)
          return sk->sk_prot->recvmsg(sk, msg, size, flags & MSG_DONTWAIT,
                                      flags & ~MSG_DONTWAIT, NULL);
  

#endif

KASAN might be unhappy about access to sk->sk_prot not annotated with READ_ONCE. In unix_bpf we have WRITE_ONCE(sk->sk_prot, ...) [1]

Got it. Will check and add READ_ONCE if necessary.

[...]

[1] https://github.com/google/ktsan/wiki/READ_ONCE-and-WRITE_ONCE#why-kernel-cod...