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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAAVpQUAfTkuqBsad+TcetvbbkidmA9pu0F+TRir==Czd9ZV4gQ@mail.gmail.com>
Date: Wed, 2 Jul 2025 10:17:43 -0700
From: Kuniyuki Iwashima <kuniyu@...gle.com>
To: Eric Dumazet <edumazet@...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 Wed, Jul 2, 2025 at 1:03 AM Eric Dumazet <edumazet@...gle.com> wrote:
>
> 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 ?

Ah right.

clip's vcc->user_back is expected to be freed by calling
clip_push() with NULL from vcc_destroy_socket(), but
atm_init_atmarp() sets NULL to ->push, and memory will
be leaked.

I'll add another patch to prevent setting ATMARPD_CTRL
after ATMARP_MKIP.

Thanks!