Re: [PATCH] atm: lec: fix use-after-free in sock_def_readable()

[Eric Dumazet]

On Mon, Mar 9, 2026 at 10:36 AM Deepanshu Kartikey
<kartikey406@xxxxxxxxx> wrote:
>
> A race condition exists between lec_atm_close() setting priv->lecd = NULL
> and concurrent access to priv->lecd in send_to_lecd(), lec_handle_bridge(),
> and lec_atm_send(). When the socket is freed via RCU while another thread
> is still using it, a use-after-free occurs in sock_def_readable() when
> accessing the socket's wait queue.
>
> The root cause is that lec_atm_close() clears priv->lecd without holding
> lec_arp_lock, while callers dereference priv->lecd without any protection
> against concurrent teardown.
>
> Fix this by:
> - Protecting priv->lecd = NULL in lec_atm_close() with lec_arp_lock to
>   synchronize with callers that already hold the lock (e.g. lec_arp_resolve)
> - Using sock_hold/sock_put in send_to_lecd() to pin the socket while in
>   use. This is safe because send_to_lecd() is called under lec_arp_lock
>   by lec_arp_resolve(), preventing concurrent NULL assignment of lecd.
> - Using lec_arp_lock + sock_hold/sock_put in lec_handle_bridge() and
>   lec_atm_send() where the lock is not held by the caller, with proper
>   skb cleanup on early exit to avoid memory leaks.
>
> Note: Patch testing via syzbot was attempted but the test VM crashed
> due to a QEMU AHCI emulation assertion failure (hw/ide/core.c:934)
> unrelated to this fix. The QEMU crash is caused by syzbot's disk I/O
> fuzzing triggering a known QEMU NCQ emulation bug, not by this patch.
> Compile testing with "make W=1 net/atm/lec.o" passes cleanly.
>
> Reported-by: syzbot+f50072212ab792c86925@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=f50072212ab792c86925
> Signed-off-by: Deepanshu Kartikey <kartikey406@xxxxxxxxx>
> ---
>  net/atm/lec.c | 31 +++++++++++++++++++++++++++----
>  1 file changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/net/atm/lec.c b/net/atm/lec.c
> index fb93c6e1c329..7e051174a92b 100644
> --- a/net/atm/lec.c
> +++ b/net/atm/lec.c
> @@ -131,6 +131,7 @@ static void lec_handle_bridge(struct sk_buff *skb, struct net_device *dev)
>  {
>         char *buff;
>         struct lec_priv *priv;
> +       unsigned long flags;
>
>         /*
>          * Check if this is a BPDU. If so, ask zeppelin to send
> @@ -154,10 +155,19 @@ static void lec_handle_bridge(struct sk_buff *skb, struct net_device *dev)
>                                         /* 0x01 is topology change */
>
>                 priv = netdev_priv(dev);
> -               atm_force_charge(priv->lecd, skb2->truesize);
> +               spin_lock_irqsave(&priv->lec_arp_lock, flags);
> +               if (!priv->lecd) {
> +                       spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
> +                       kfree_skb(skb2);
> +                       return;
> +               }
>                 sk = sk_atm(priv->lecd);
> +               sock_hold(sk);
> +               spin_unlock_irqrestore(&priv->lec_arp_lock, flags);

What prevents priv->lecd to be NULL after you released priv->lec_arp_lock ?


> +               atm_force_charge(priv->lecd, skb2->truesize);
>                 skb_queue_tail(&sk->sk_receive_queue, skb2);
>                 sk->sk_data_ready(sk);
> +               sock_put(sk);
>         }
>  }
>  #endif /* IS_ENABLED(CONFIG_BRIDGE) */
> @@ -441,7 +451,7 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
>                         /* hit from bridge table, send LE_ARP_RESPONSE */
>                         struct sk_buff *skb2;
>                         struct sock *sk;
> -
> +                       unsigned long flags;
>                         pr_debug("%s: entry found, responding to zeppelin\n",
>                                  dev->name);
>                         skb2 = alloc_skb(sizeof(struct atmlec_msg), GFP_ATOMIC);
> @@ -449,10 +459,19 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
>                                 break;
>                         skb2->len = sizeof(struct atmlec_msg);
>                         skb_copy_to_linear_data(skb2, mesg, sizeof(*mesg));
> -                       atm_force_charge(priv->lecd, skb2->truesize);
> +                       spin_lock_irqsave(&priv->lec_arp_lock, flags);
> +                       if (!priv->lecd) {
> +                               spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
> +                               kfree_skb(skb2);
> +                               break;
> +                       }
>                         sk = sk_atm(priv->lecd);
> +                       sock_hold(sk);
> +                       spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
> +                       atm_force_charge(priv->lecd, skb2->truesize);
>                         skb_queue_tail(&sk->sk_receive_queue, skb2);
>                         sk->sk_data_ready(sk);
> +                       sock_put(sk);
>                 }
>         }
>  #endif /* IS_ENABLED(CONFIG_BRIDGE) */
> @@ -471,8 +490,11 @@ static void lec_atm_close(struct atm_vcc *vcc)
>         struct sk_buff *skb;
>         struct net_device *dev = (struct net_device *)vcc->proto_data;
>         struct lec_priv *priv = netdev_priv(dev);
> +       unsigned long flags;
>
> +       spin_lock_irqsave(&priv->lec_arp_lock, flags);
>         priv->lecd = NULL;
> +       spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
>         /* Do something needful? */
>
>         netif_stop_queue(dev);
> @@ -534,6 +556,7 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
>
>         atm_force_charge(priv->lecd, skb->truesize);
>         sk = sk_atm(priv->lecd);
> +       sock_hold(sk);
>         skb_queue_tail(&sk->sk_receive_queue, skb);
>         sk->sk_data_ready(sk);
>
> @@ -543,7 +566,7 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
>                 skb_queue_tail(&sk->sk_receive_queue, data);
>                 sk->sk_data_ready(sk);
>         }
> -
> +       sock_put(sk);
>         return 0;
>  }
>
> --
> 2.43.0
>