Skip to content

Commit

Permalink
ax25: NPD bug when detaching AX25 device
Browse files Browse the repository at this point in the history
The existing cleanup routine implementation is not well synchronized
with the syscall routine. When a device is detaching, below race could
occur.

static int ax25_sendmsg(...) {
  ...
  lock_sock()
  ax25 = sk_to_ax25(sk);
  if (ax25->ax25_dev == NULL) // CHECK
  ...
  ax25_queue_xmit(skb, ax25->ax25_dev->dev); // USE
  ...
}

static void ax25_kill_by_device(...) {
  ...
  if (s->ax25_dev == ax25_dev) {
    s->ax25_dev = NULL;
    ...
}

Other syscall functions like ax25_getsockopt, ax25_getname,
ax25_info_show also suffer from similar races. To fix them, this patch
introduce lock_sock() into ax25_kill_by_device in order to guarantee
that the nullify action in cleanup routine cannot proceed when another
socket request is pending.

Signed-off-by: Hanjie Wu <nagi@zju.edu.cn>
Signed-off-by: Lin Ma <linma@zju.edu.cn>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Lin Ma authored and David S. Miller committed Dec 18, 2021
1 parent b2f37ae commit 1ade48d
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion net/ax25/af_ax25.c
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,10 @@ static void ax25_kill_by_device(struct net_device *dev)
again:
ax25_for_each(s, &ax25_list) {
if (s->ax25_dev == ax25_dev) {
s->ax25_dev = NULL;
spin_unlock_bh(&ax25_list_lock);
lock_sock(s->sk);
s->ax25_dev = NULL;
release_sock(s->sk);
ax25_disconnect(s, ENETUNREACH);
spin_lock_bh(&ax25_list_lock);

Expand Down

0 comments on commit 1ade48d

Please sign in to comment.