[PATCH net] appletalk: Hold socket reference in atalk_rcv()

[PATCH net] appletalk: Hold socket reference in atalk_rcv()

[Yizhou Zhao]

atalk_search_socket() walks the global atalk_sockets list while holding
atalk_sockets_lock, but it returns the matching socket after dropping the
lock without taking a reference.  atalk_rcv() then passes that pointer to
sock_queue_rcv_skb().

That leaves a race with close().  A concurrent atalk_release() can orphan
the socket, remove it from atalk_sockets, and drop the final reference via
atalk_destroy_socket(), freeing the socket before atalk_rcv() queues the
incoming skb.

On a KASAN-enabled kernel this can be reproduced by racing AppleTalk DDP
delivery on loopback against close/rebind of the destination DGRAM socket:

  BUG: KASAN: slab-use-after-free in selinux_socket_sock_rcv_skb()
  sk_filter_trim_cap()
  sock_queue_rcv_skb_reason()
  atalk_rcv()
  snap_rcv()
  llc_rcv()

Take a reference on the selected socket before dropping
atalk_sockets_lock, and put it after sock_queue_rcv_skb() has finished.
This keeps the socket alive for the receive path without changing socket
lookup semantics.  A malformed or racing receive still drops the skb on
queueing failure as before.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Reported-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
Reported-by: Yuxiang Yang <yangyx22@mails.tsinghua.edu.cn>
Reported-by: Ao Wang <wangao@seu.edu.cn>
Reported-by: Xuewei Feng <fengxw06@126.com>
Reported-by: Qi Li <qli01@tsinghua.edu.cn>
Reported-by: Ke Xu <xuke@tsinghua.edu.cn>
Assisted-by: GLM:GLM-5.1
Signed-off-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
---
 net/appletalk/ddp.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
index 30a6dc06291c..61ec5c569dc3 100644
--- a/net/appletalk/ddp.c
+++ b/net/appletalk/ddp.c
@@ -131,6 +131,8 @@ static struct sock *atalk_search_socket(struct sockaddr_at *to,
 	}
 	s = def_socket;
 found:
+	if (s)
+		sock_hold(s);
 	read_unlock_bh(&atalk_sockets_lock);
 	return s;
 }
@@ -1474,9 +1476,12 @@ static int atalk_rcv(struct sk_buff *skb, struct net_device *dev,
 		goto drop;
 
 	/* Queue packet (standard) */
-	if (sock_queue_rcv_skb(sock, skb) < 0)
+	if (sock_queue_rcv_skb(sock, skb) < 0) {
+		sock_put(sock);
 		goto drop;
+	}
 
+	sock_put(sock);
 	return NET_RX_SUCCESS;
 
 drop:
-- 
2.43.0

Re: [PATCH net] appletalk: Hold socket reference in atalk_rcv()

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
appletalk: Hold socket reference in atalk_rcv()

This commit addresses a use-after-free race by holding a reference to the
socket found by atalk_search_socket() until after the packet is queued in
atalk_rcv().

> diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
> index 30a6dc06291c3..61ec5c569dc3c 100644
> --- a/net/appletalk/ddp.c
> +++ b/net/appletalk/ddp.c

[ ... ]

> @@ -1474,9 +1476,12 @@ static int atalk_rcv(struct sk_buff *skb, struct net_device *dev,
>  		goto drop;
>  
>  	/* Queue packet (standard) */
> -	if (sock_queue_rcv_skb(sock, skb) < 0)
> +	if (sock_queue_rcv_skb(sock, skb) < 0) {
> +		sock_put(sock);
>  		goto drop;
> +	}

[Severity: High]
Does this code introduce a memory leak or an infinite timer loop?

By holding a socket reference in atalk_rcv(), the socket memory is kept alive,
but atalk_rcv() does not hold the socket lock. A concurrent atalk_release()
could lock the socket, mark it SOCK_DEAD, and call atalk_destroy_socket(),
which logically purges the receive queue:

net/appletalk/ddp.c:atalk_destroy_socket() {
    ...
    skb_queue_purge(&sk->sk_receive_queue);
    ...
}

After this purge, atalk_rcv() might call sock_queue_rcv_skb(). The core
function __sock_queue_rcv_skb() does not reject packets for SOCK_DEAD sockets.
It will successfully enqueue the skb and increment sk->sk_rmem_alloc.

Since AppleTalk uses the empty sock_def_destruct() instead of a custom
sk_destruct callback, is the receive queue ever purged upon final sk_free(),
or does this permanently leak the skb?

Furthermore, if atalk_destroy_socket() finds other pending allocations and
arms the destruction timer, will the timer endlessly re-arm itself?

net/appletalk/ddp.c:atalk_destroy_timer() {
    ...
    if (sk_has_allocations(sk)) {
        sk->sk_timer.expires = jiffies + SOCK_DESTROY_TIME;
        add_timer(&sk->sk_timer);
    }
    ...
}

The timer checks sk_has_allocations(sk), which will remain true indefinitely
due to the uncleared sk_rmem_alloc from the leaked skb.

>  
> +	sock_put(sock);
>  	return NET_RX_SUCCESS;

Re: [PATCH net] appletalk: Hold socket reference in atalk_rcv()

[Eric Dumazet]

On Sun, Jun 14, 2026 at 2:52 AM Yizhou Zhao
<zhaoyz24@mails.tsinghua.edu.cn> wrote:
>
> atalk_search_socket() walks the global atalk_sockets list while holding
> atalk_sockets_lock, but it returns the matching socket after dropping the
> lock without taking a reference.  atalk_rcv() then passes that pointer to
> sock_queue_rcv_skb().
>
> That leaves a race with close().  A concurrent atalk_release() can orphan
> the socket, remove it from atalk_sockets, and drop the final reference via
> atalk_destroy_socket(), freeing the socket before atalk_rcv() queues the
> incoming skb.
>
> On a KASAN-enabled kernel this can be reproduced by racing AppleTalk DDP
> delivery on loopback against close/rebind of the destination DGRAM socket:
>
>   BUG: KASAN: slab-use-after-free in selinux_socket_sock_rcv_skb()
>   sk_filter_trim_cap()
>   sock_queue_rcv_skb_reason()
>   atalk_rcv()
>   snap_rcv()
>   llc_rcv()
>
> Take a reference on the selected socket before dropping
> atalk_sockets_lock, and put it after sock_queue_rcv_skb() has finished.
> This keeps the socket alive for the receive path without changing socket
> lookup semantics.  A malformed or racing receive still drops the skb on
> queueing failure as before.

No idea why linux still carries appletalk.

MacOS dropped it 20 years ago.

Re: [PATCH net] appletalk: Hold socket reference in atalk_rcv()

[Jakub Kicinski]

On Mon, 15 Jun 2026 09:53:59 -0700 Eric Dumazet wrote:
> > atalk_search_socket() walks the global atalk_sockets list while holding
> > atalk_sockets_lock, but it returns the matching socket after dropping the
> > lock without taking a reference.  atalk_rcv() then passes that pointer to
> > sock_queue_rcv_skb().
> >
> > That leaves a race with close().  A concurrent atalk_release() can orphan
> > the socket, remove it from atalk_sockets, and drop the final reference via
> > atalk_destroy_socket(), freeing the socket before atalk_rcv() queues the
> > incoming skb.
> >
> > On a KASAN-enabled kernel this can be reproduced by racing AppleTalk DDP
> > delivery on loopback against close/rebind of the destination DGRAM socket:
> >
> >   BUG: KASAN: slab-use-after-free in selinux_socket_sock_rcv_skb()
> >   sk_filter_trim_cap()
> >   sock_queue_rcv_skb_reason()
> >   atalk_rcv()
> >   snap_rcv()
> >   llc_rcv()
> >
> > Take a reference on the selected socket before dropping
> > atalk_sockets_lock, and put it after sock_queue_rcv_skb() has finished.
> > This keeps the socket alive for the receive path without changing socket
> > lookup semantics.  A malformed or racing receive still drops the skb on
> > queueing failure as before.  
> 
> No idea why linux still carries appletalk.
> 
> MacOS dropped it 20 years ago.

Yes. Let me try to move it to mod-orphan.