Commit d6ab640c authored by Jakub Kicinski's avatar Jakub Kicinski

Merge branch 'netlink-annotate-various-data-races'

Eric Dumazet says:

====================
netlink: annotate various data races

A recent syzbot report came to my attention.

After addressing it, I also fixed other related races.
====================

Link: https://lore.kernel.org/r/20230120125955.3453768-1-edumazet@google.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents 9f535c87 9b663b5c
...@@ -580,7 +580,9 @@ static int netlink_insert(struct sock *sk, u32 portid) ...@@ -580,7 +580,9 @@ static int netlink_insert(struct sock *sk, u32 portid)
if (nlk_sk(sk)->bound) if (nlk_sk(sk)->bound)
goto err; goto err;
nlk_sk(sk)->portid = portid; /* portid can be read locklessly from netlink_getname(). */
WRITE_ONCE(nlk_sk(sk)->portid, portid);
sock_hold(sk); sock_hold(sk);
err = __netlink_insert(table, sk); err = __netlink_insert(table, sk);
...@@ -1096,9 +1098,11 @@ static int netlink_connect(struct socket *sock, struct sockaddr *addr, ...@@ -1096,9 +1098,11 @@ static int netlink_connect(struct socket *sock, struct sockaddr *addr,
return -EINVAL; return -EINVAL;
if (addr->sa_family == AF_UNSPEC) { if (addr->sa_family == AF_UNSPEC) {
sk->sk_state = NETLINK_UNCONNECTED; /* paired with READ_ONCE() in netlink_getsockbyportid() */
nlk->dst_portid = 0; WRITE_ONCE(sk->sk_state, NETLINK_UNCONNECTED);
nlk->dst_group = 0; /* dst_portid and dst_group can be read locklessly */
WRITE_ONCE(nlk->dst_portid, 0);
WRITE_ONCE(nlk->dst_group, 0);
return 0; return 0;
} }
if (addr->sa_family != AF_NETLINK) if (addr->sa_family != AF_NETLINK)
...@@ -1119,9 +1123,11 @@ static int netlink_connect(struct socket *sock, struct sockaddr *addr, ...@@ -1119,9 +1123,11 @@ static int netlink_connect(struct socket *sock, struct sockaddr *addr,
err = netlink_autobind(sock); err = netlink_autobind(sock);
if (err == 0) { if (err == 0) {
sk->sk_state = NETLINK_CONNECTED; /* paired with READ_ONCE() in netlink_getsockbyportid() */
nlk->dst_portid = nladdr->nl_pid; WRITE_ONCE(sk->sk_state, NETLINK_CONNECTED);
nlk->dst_group = ffs(nladdr->nl_groups); /* dst_portid and dst_group can be read locklessly */
WRITE_ONCE(nlk->dst_portid, nladdr->nl_pid);
WRITE_ONCE(nlk->dst_group, ffs(nladdr->nl_groups));
} }
return err; return err;
...@@ -1138,10 +1144,12 @@ static int netlink_getname(struct socket *sock, struct sockaddr *addr, ...@@ -1138,10 +1144,12 @@ static int netlink_getname(struct socket *sock, struct sockaddr *addr,
nladdr->nl_pad = 0; nladdr->nl_pad = 0;
if (peer) { if (peer) {
nladdr->nl_pid = nlk->dst_portid; /* Paired with WRITE_ONCE() in netlink_connect() */
nladdr->nl_groups = netlink_group_mask(nlk->dst_group); nladdr->nl_pid = READ_ONCE(nlk->dst_portid);
nladdr->nl_groups = netlink_group_mask(READ_ONCE(nlk->dst_group));
} else { } else {
nladdr->nl_pid = nlk->portid; /* Paired with WRITE_ONCE() in netlink_insert() */
nladdr->nl_pid = READ_ONCE(nlk->portid);
netlink_lock_table(); netlink_lock_table();
nladdr->nl_groups = nlk->groups ? nlk->groups[0] : 0; nladdr->nl_groups = nlk->groups ? nlk->groups[0] : 0;
netlink_unlock_table(); netlink_unlock_table();
...@@ -1168,8 +1176,9 @@ static struct sock *netlink_getsockbyportid(struct sock *ssk, u32 portid) ...@@ -1168,8 +1176,9 @@ static struct sock *netlink_getsockbyportid(struct sock *ssk, u32 portid)
/* Don't bother queuing skb if kernel socket has no input function */ /* Don't bother queuing skb if kernel socket has no input function */
nlk = nlk_sk(sock); nlk = nlk_sk(sock);
if (sock->sk_state == NETLINK_CONNECTED && /* dst_portid and sk_state can be changed in netlink_connect() */
nlk->dst_portid != nlk_sk(ssk)->portid) { if (READ_ONCE(sock->sk_state) == NETLINK_CONNECTED &&
READ_ONCE(nlk->dst_portid) != nlk_sk(ssk)->portid) {
sock_put(sock); sock_put(sock);
return ERR_PTR(-ECONNREFUSED); return ERR_PTR(-ECONNREFUSED);
} }
...@@ -1886,8 +1895,9 @@ static int netlink_sendmsg(struct socket *sock, struct msghdr *msg, size_t len) ...@@ -1886,8 +1895,9 @@ static int netlink_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
goto out; goto out;
netlink_skb_flags |= NETLINK_SKB_DST; netlink_skb_flags |= NETLINK_SKB_DST;
} else { } else {
dst_portid = nlk->dst_portid; /* Paired with WRITE_ONCE() in netlink_connect() */
dst_group = nlk->dst_group; dst_portid = READ_ONCE(nlk->dst_portid);
dst_group = READ_ONCE(nlk->dst_group);
} }
/* Paired with WRITE_ONCE() in netlink_insert() */ /* Paired with WRITE_ONCE() in netlink_insert() */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment