Skip to content

Commit

Permalink
netlink: Fix autobind race condition that leads to zero port ID
Browse files Browse the repository at this point in the history
The commit c0bb07d ("netlink:
Reset portid after netlink_insert failure") introduced a race
condition where if two threads try to autobind the same socket
one of them may end up with a zero port ID.  This led to kernel
deadlocks that were observed by multiple people.

This patch reverts that commit and instead fixes it by introducing
a separte rhash_portid variable so that the real portid is only set
after the socket has been successfully hashed.

Fixes: c0bb07d ("netlink: Reset portid after netlink_insert failure")
Reported-by: Tejun Heo <[email protected]>
Reported-by: Linus Torvalds <[email protected]>
Signed-off-by: Herbert Xu <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
  • Loading branch information
herbertx authored and davem330 committed Sep 21, 2015
1 parent 3ea7924 commit 1f770c0
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 5 deletions.
12 changes: 7 additions & 5 deletions net/netlink/af_netlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -1031,7 +1031,7 @@ static inline int netlink_compare(struct rhashtable_compare_arg *arg,
const struct netlink_compare_arg *x = arg->key;
const struct netlink_sock *nlk = ptr;

return nlk->portid != x->portid ||
return nlk->rhash_portid != x->portid ||
!net_eq(sock_net(&nlk->sk), read_pnet(&x->pnet));
}

Expand All @@ -1057,7 +1057,7 @@ static int __netlink_insert(struct netlink_table *table, struct sock *sk)
{
struct netlink_compare_arg arg;

netlink_compare_arg_init(&arg, sock_net(sk), nlk_sk(sk)->portid);
netlink_compare_arg_init(&arg, sock_net(sk), nlk_sk(sk)->rhash_portid);
return rhashtable_lookup_insert_key(&table->hash, &arg,
&nlk_sk(sk)->node,
netlink_rhashtable_params);
Expand Down Expand Up @@ -1119,7 +1119,7 @@ static int netlink_insert(struct sock *sk, u32 portid)
unlikely(atomic_read(&table->hash.nelems) >= UINT_MAX))
goto err;

nlk_sk(sk)->portid = portid;
nlk_sk(sk)->rhash_portid = portid;
sock_hold(sk);

err = __netlink_insert(table, sk);
Expand All @@ -1131,10 +1131,12 @@ static int netlink_insert(struct sock *sk, u32 portid)
err = -EOVERFLOW;
if (err == -EEXIST)
err = -EADDRINUSE;
nlk_sk(sk)->portid = 0;
sock_put(sk);
goto err;
}

nlk_sk(sk)->portid = portid;

err:
release_sock(sk);
return err;
Expand Down Expand Up @@ -3271,7 +3273,7 @@ static inline u32 netlink_hash(const void *data, u32 len, u32 seed)
const struct netlink_sock *nlk = data;
struct netlink_compare_arg arg;

netlink_compare_arg_init(&arg, sock_net(&nlk->sk), nlk->portid);
netlink_compare_arg_init(&arg, sock_net(&nlk->sk), nlk->rhash_portid);
return jhash2((u32 *)&arg, netlink_compare_arg_len / sizeof(u32), seed);
}

Expand Down
1 change: 1 addition & 0 deletions net/netlink/af_netlink.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ struct netlink_ring {
struct netlink_sock {
/* struct sock has to be the first member of netlink_sock */
struct sock sk;
u32 rhash_portid;
u32 portid;
u32 dst_portid;
u32 dst_group;
Expand Down

0 comments on commit 1f770c0

Please sign in to comment.