netdev - Re: [PATCH v1 net 1/2] atm: clip: Fix infinite recursive call of clip

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89iKmA41ERK2VFScyrJ7PNNwqH4VBK9kpzNgxO3oFTRq=mQ@mail.gmail.com>
Date: Wed, 2 Jul 2025 01:02:51 -0700
From: Eric Dumazet <edumazet@...gle.com>
To: Kuniyuki Iwashima <kuniyu@...gle.com>
Cc: "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, 
	Kuniyuki Iwashima <kuni1840@...il.com>, netdev@...r.kernel.org, 
	syzbot+0c77cccd6b7cd917b35a@...kaller.appspotmail.com
Subject: Re: [PATCH v1 net 1/2] atm: clip: Fix infinite recursive call of clip_push().

On Tue, Jul 1, 2025 at 7:04 PM Kuniyuki Iwashima <kuniyu@...gle.com> wrote:
>
> syzbot reported the splat below. [0]
>
> This happens if we call ioctl(ATMARP_MKIP) more than once.
>
> During the first call, clip_mkip() sets clip_push() to vcc->push(),
> and the second call copies it to clip_vcc->old_push().
>
> Later, when a NULL skb is passed to clip_push(), it calls
> clip_vcc->old_push(), triggering the infinite recursion.
>
> Let's prevent the second ioctl(ATMARP_MKIP) by checking
> vcc->user_back, which is allocated by the first call as clip_vcc.
>
> Note also that we use lock_sock() to prevent racy calls.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: syzbot+0c77cccd6b7cd917b35a@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=2371d94d248d126c1eb1
> Signed-off-by: Kuniyuki Iwashima <kuniyu@...gle.com>
> ---
>  net/atm/clip.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/net/atm/clip.c b/net/atm/clip.c
> index b234dc3bcb0d..250b3c7f4305 100644
> --- a/net/atm/clip.c
> +++ b/net/atm/clip.c
> @@ -417,6 +417,8 @@ static int clip_mkip(struct atm_vcc *vcc, int timeout)
>
>         if (!vcc->push)
>                 return -EBADFD;
> +       if (vcc->user_back)
> +               return -EINVAL;
>         clip_vcc = kmalloc(sizeof(struct clip_vcc), GFP_KERNEL);
>         if (!clip_vcc)
>                 return -ENOMEM;
> @@ -655,6 +657,7 @@ static int atm_init_atmarp(struct atm_vcc *vcc)
>  static int clip_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
>  {
>         struct atm_vcc *vcc = ATM_SD(sock);
> +       struct sock *sk = sock->sk;
>         int err = 0;
>
>         switch (cmd) {
> @@ -682,7 +685,9 @@ static int clip_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
>                 }
>                 break;
>         case ATMARP_MKIP:
> +               lock_sock(sk);
>                 err = clip_mkip(vcc, arg);
> +               release_sock(sk);

This will still race with atm_init_atmarp(), which (ab)uses RTNL ?

>                 break;
>         case ATMARP_SETENTRY:
>                 err = clip_setentry(vcc, (__force __be32)arg);
> --
> 2.50.0.727.gbf7dc18ff4-goog
>