Re: [PATCH net] net: atm: fix use-after-free in sigd_put_skb()

[Weiming Shi]

On Tue Jun 9, 2026 at 11:49 PM CST, Weiming Shi wrote:
> On 26-06-09 13:06, Paolo Abeni wrote:
>> From: AI Reviewer <ai@xxxxxxxxxxx>
>> 
>> 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/
>> ---
>> net: atm: fix use-after-free in sigd_put_skb()
>> 
>> This commit addresses a use-after-free in sigd_put_skb() by pinning the
>> socket with find_get_vcc() and checking if it is marked SOCK_DEAD under
>> the callback lock.
>> 
>> > diff --git a/net/atm/signaling.c b/net/atm/signaling.c
>> > index b991d937205af..3dbe8e0fdc9af 100644
>> > --- a/net/atm/signaling.c
>> > +++ b/net/atm/signaling.c
>> > @@ -54,14 +54,31 @@ static struct atm_vcc *find_get_vcc(struct atm_vcc *vcc)
>> >  
>> >  static void sigd_put_skb(struct sk_buff *skb)
>> >  {
>> > -	if (!sigd) {
>> > +	struct atm_vcc *vcc;
>> > +	struct sock *sk;
>> > +
>> > +	vcc = find_get_vcc(READ_ONCE(sigd));
>> 
>> [Severity: Critical]
>> Does this introduce a regression where an ABA vulnerability allows a KASLR
>> bypass if the daemon socket is freed and reallocated?
>> 
>> If the signaling daemon is closed, sigd is cleared and the socket freed, but
>> an unprivileged thread in sigd_put_skb() might have already latched the old
>> sigd pointer.
>> 
>> If an unprivileged user subsequently creates a new ATM socket, couldn't the
>> slab allocator reuse the exact same memory address? In this case, find_get_vcc()
>> would match the raw pointer and return the attacker's new socket.
>> 
>> Since the attacker's socket isn't marked SOCK_DEAD, sigd_put_skb() would blindly
>> enqueue the signaling message to the attacker's receive queue.
>> 
>> Because the message contains literal kernel pointers (vcc, listen_vcc),
>> wouldn't this allow an unprivileged KASLR bypass and message injection?
>> 
>> [ ... ]
>> 
>> [Severity: High]
>> This is a pre-existing issue, but does sigd_send() leak memory when queuing
>> SKBs to a closing listening socket?
>> 
>> When processing an as_indicate message, sigd_send() resolves the listening
>> socket via msg->listen_vcc, acquires lock_sock(), and appends an SKB:
>> 
>> net/atm/signaling.c:sigd_send() {
>>     ...
>>     lock_sock(sk);
>>     if (sk_acceptq_is_full(sk)) {
>>         ...
>>     }
>>     sk_acceptq_added(sk);
>>     skb_queue_tail(&sk->sk_receive_queue, skb);
>>     ...
>> }
>> 
>> Does this code blindly queue an SKB without checking if the socket has been
>> marked closed (ATM_VF_CLOSE) and already purged?
>> 
>> If the listening socket is concurrently closed, vcc_release() will lock the
>> socket, purge its receive queue, mark it closed, and release the lock.
>> 
>> Could sigd_send() then acquire the lock and append the new SKB to the dead
>> socket's receive queue? Since sk_free() expects an empty queue and does not
>> purge it during memory free, would this permanently leak the SKB and allow
>> an attacker to exhaust kernel memory?
>> -- 
>> This is an AI-generated review.
>> 
>
> Thanks. The find_get_vcc() issue will be fixed in v2; the sigd_send()
> leak will be sent as a separate patch.

Hi Paolo,

This is the link to the fix for the above bug.

v2: https://lore.kernel.org/all/DJ6D0B3KSMYU.1KE7UIQCA7K2N@xxxxxxxxx/
leak memory fix: https://lore.kernel.org/all/20260611163805.2151734-2-bestswngs@xxxxxxxxx/

Best,
Weiming Shi