6.18-stable review patch. If anyone has any objections, please let me know. ------------------ From: Bernard Pidoux ax25_kill_by_device() detaches a connection from a device that goes down by setting s->ax25_dev to NULL. It does so after it has dropped ax25_list_lock, while the connection is still on ax25_list. __ax25_find_cb() and ax25_find_listener() walk that list and test s->ax25_dev against NULL before they dereference it, but they load the pointer a second time for the dereference. ax25_send_to_raw() dereferences it without any test. If ax25_kill_by_device() clears the pointer in between, the walker dereferences NULL: Oops: general protection fault, probably for non-canonical address KASAN: null-ptr-deref in range [0x0000000000000010-0x0000000000000017] RIP: 0010:ax25_find_cb+0x121/0x330 [ax25] ax25_send_frame+0xdc/0x980 [ax25] rose_send_frame+0xce/0x270 [rose] rose_transmit_restart_request+0x1b3/0x260 [rose] rose_transmit_link+0x31d/0x560 [rose] rose_write_internal+0x3ab/0x15b0 [rose] rose_release+0x2b2/0x4b0 [rose] Load the pointer once in the three walkers and use that value, and add the missing NULL test to ax25_send_to_raw(). The ax25_dev itself cannot go away under the walker: ax25_kill_by_device() has to take ax25_list_lock again, in ax25_cb_del(), before the device teardown gets as far as freeing it. The oops above was obtained on 6.12.94 with KASAN, after 382 seconds of the syzbot C reproducer of the ax25_find_cb() use-after-free report, with the interface brought up again in a loop, while testing an earlier version of the next patch. With this series the same test runs for 600 seconds without an oops. Signed-off-by: Bernard Pidoux Assisted-by: Claude:claude-opus-5-5 (cherry picked from commit 3696f3cf8690e335a2cc7e8103f018bc205de738) Signed-off-by: Sasha Levin --- net/ax25/af_ax25.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c index 28b7a89afaf6e..76f6848fba7d6 100644 --- a/net/ax25/af_ax25.c +++ b/net/ax25/af_ax25.c @@ -91,7 +91,7 @@ static void ax25_kill_by_device(struct net_device *dev) if (!sk) { spin_unlock_bh(&ax25_list_lock); ax25_disconnect(s, ENETUNREACH); - s->ax25_dev = NULL; + WRITE_ONCE(s->ax25_dev, NULL); ax25_cb_del(s); spin_lock_bh(&ax25_list_lock); goto again; @@ -100,7 +100,7 @@ static void ax25_kill_by_device(struct net_device *dev) spin_unlock_bh(&ax25_list_lock); lock_sock(sk); ax25_disconnect(s, ENETUNREACH); - s->ax25_dev = NULL; + WRITE_ONCE(s->ax25_dev, NULL); if (sk->sk_socket) { netdev_put(ax25_dev->dev, &s->dev_tracker); @@ -172,6 +172,7 @@ void ax25_cb_add(ax25_cb *ax25) struct sock *ax25_find_listener(ax25_address *addr, int digi, struct net_device *dev, int type) { + ax25_dev *ax25_dev; ax25_cb *s; spin_lock(&ax25_list_lock); @@ -181,7 +182,8 @@ struct sock *ax25_find_listener(ax25_address *addr, int digi, if (s->sk && !ax25cmp(&s->source_addr, addr) && s->sk->sk_type == type && s->sk->sk_state == TCP_LISTEN) { /* If device is null we match any device */ - if (s->ax25_dev == NULL || s->ax25_dev->dev == dev) { + ax25_dev = READ_ONCE(s->ax25_dev); + if (ax25_dev == NULL || ax25_dev->dev == dev) { sock_hold(s->sk); spin_unlock(&ax25_list_lock); return s->sk; @@ -226,14 +228,16 @@ static ax25_cb *__ax25_find_cb(const ax25_address *src_addr, ax25_address *dest_addr, ax25_digi *digi, struct net_device *dev) { + ax25_dev *ax25_dev; ax25_cb *s; ax25_for_each(s, &ax25_list) { if (s->sk && s->sk->sk_type != SOCK_SEQPACKET) continue; - if (s->ax25_dev == NULL) + ax25_dev = READ_ONCE(s->ax25_dev); + if (ax25_dev == NULL) continue; - if (ax25cmp(&s->source_addr, src_addr) == 0 && ax25cmp(&s->dest_addr, dest_addr) == 0 && s->ax25_dev->dev == dev) { + if (ax25cmp(&s->source_addr, src_addr) == 0 && ax25cmp(&s->dest_addr, dest_addr) == 0 && ax25_dev->dev == dev) { if (digi != NULL && digi->ndigi != 0) { if (s->digipeat == NULL) continue; @@ -293,15 +297,17 @@ ax25_cb *ax25_cb_add_unique(ax25_cb *ax25) void ax25_send_to_raw(ax25_address *addr, struct sk_buff *skb, int proto) { + ax25_dev *ax25_dev; ax25_cb *s; struct sk_buff *copy; spin_lock(&ax25_list_lock); ax25_for_each(s, &ax25_list) { + ax25_dev = READ_ONCE(s->ax25_dev); if (s->sk != NULL && ax25cmp(&s->source_addr, addr) == 0 && s->sk->sk_type == SOCK_RAW && s->sk->sk_protocol == proto && - s->ax25_dev->dev == skb->dev && + ax25_dev && ax25_dev->dev == skb->dev && atomic_read(&s->sk->sk_rmem_alloc) <= s->sk->sk_rcvbuf) { if ((copy = skb_clone(skb, GFP_ATOMIC)) == NULL) continue; -- 2.53.0