[PATCH v1 net 0/2] atm: clip: Fix infinite recursion and potential null-ptr-deref.

[PATCH v1 net 0/2] atm: clip: Fix infinite recursion and potential null-ptr-deref.

[Kuniyuki Iwashima]

Patch 1 fixes infinite recursive call of clip_vcc->old_push(), which
was reported by syzbot.

Patch 2 fixes racy access to atmarpd found while checking RTNL usage
in clip.c.


Kuniyuki Iwashima (2):
  atm: clip: Fix infinite recursive call of clip_push().
  atm: clip: Fix potential null-ptr-deref in to_atmarpd().

 net/atm/clip.c | 47 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 14 deletions(-)

-- 
2.50.0.727.gbf7dc18ff4-goog

[PATCH v1 net 1/2] atm: clip: Fix infinite recursive call of clip_push().

[Kuniyuki Iwashima]

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.

[0]:
BUG: TASK stack guard page was hit at ffffc9000d66fff8 (stack is ffffc9000d670000..ffffc9000d678000)
Oops: stack guard page: 0000 [#1] SMP KASAN NOPTI
CPU: 0 UID: 0 PID: 5322 Comm: syz.0.0 Not tainted 6.16.0-rc4-syzkaller #0 PREEMPT(full)
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
RIP: 0010:clip_push+0x5/0x720 net/atm/clip.c:191
Code: e0 8f aa 8c e8 1c ad 5b fa eb ae 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 55 <41> 57 41 56 41 55 41 54 53 48 83 ec 20 48 89 f3 49 89 fd 48 bd 00
RSP: 0018:ffffc9000d670000 EFLAGS: 00010246
RAX: 1ffff1100235a4a5 RBX: ffff888011ad2508 RCX: ffff8880003c0000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff888037f01000
RBP: dffffc0000000000 R08: ffffffff8fa104f7 R09: 1ffffffff1f4209e
R10: dffffc0000000000 R11: ffffffff8a99b300 R12: ffffffff8a99b300
R13: ffff888037f01000 R14: ffff888011ad2500 R15: ffff888037f01578
FS:  000055557ab6d500(0000) GS:ffff88808d250000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffc9000d66fff8 CR3: 0000000043172000 CR4: 0000000000352ef0
Call Trace:
 <TASK>
 clip_push+0x6dc/0x720 net/atm/clip.c:200
 clip_push+0x6dc/0x720 net/atm/clip.c:200
 clip_push+0x6dc/0x720 net/atm/clip.c:200
...
 clip_push+0x6dc/0x720 net/atm/clip.c:200
 clip_push+0x6dc/0x720 net/atm/clip.c:200
 clip_push+0x6dc/0x720 net/atm/clip.c:200
 vcc_destroy_socket net/atm/common.c:183 [inline]
 vcc_release+0x157/0x460 net/atm/common.c:205
 __sock_release net/socket.c:647 [inline]
 sock_close+0xc0/0x240 net/socket.c:1391
 __fput+0x449/0xa70 fs/file_table.c:465
 task_work_run+0x1d1/0x260 kernel/task_work.c:227
 resume_user_mode_work include/linux/resume_user_mode.h:50 [inline]
 exit_to_user_mode_loop+0xec/0x110 kernel/entry/common.c:114
 exit_to_user_mode_prepare include/linux/entry-common.h:330 [inline]
 syscall_exit_to_user_mode_work include/linux/entry-common.h:414 [inline]
 syscall_exit_to_user_mode include/linux/entry-common.h:449 [inline]
 do_syscall_64+0x2bd/0x3b0 arch/x86/entry/syscall_64.c:100
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7ff31c98e929
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 a8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fffb5aa1f78 EFLAGS: 00000246 ORIG_RAX: 00000000000001b4
RAX: 0000000000000000 RBX: 0000000000012747 RCX: 00007ff31c98e929
RDX: 0000000000000000 RSI: 000000000000001e RDI: 0000000000000003
RBP: 00007ff31cbb7ba0 R08: 0000000000000001 R09: 0000000db5aa226f
R10: 00007ff31c7ff030 R11: 0000000000000246 R12: 00007ff31cbb608c
R13: 00007ff31cbb6080 R14: ffffffffffffffff R15: 00007fffb5aa2090
 </TASK>
Modules linked in:

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+0c77cccd6b7cd917b35a@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=2371d94d248d126c1eb1
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.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);
 		break;
 	case ATMARP_SETENTRY:
 		err = clip_setentry(vcc, (__force __be32)arg);
-- 
2.50.0.727.gbf7dc18ff4-goog

[PATCH v1 net 2/2] atm: clip: Fix potential null-ptr-deref in to_atmarpd().

[Kuniyuki Iwashima]

atmarpd is protected by RTNL since commit f3a0592b37b8 ("[ATM]: clip
causes unregister hang").

However, it is not enough because to_atmarpd() is called without RTNL,
especially clip_neigh_solicit() / neigh_ops->solicit() is unsleepable.

Also, there is no RTNL dependency around atmarpd.

Let's use a private mutex and RCU to protect access to atmarpd in
to_atmarpd().

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/atm/clip.c | 42 ++++++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/net/atm/clip.c b/net/atm/clip.c
index 250b3c7f4305..84d527a9398b 100644
--- a/net/atm/clip.c
+++ b/net/atm/clip.c
@@ -46,6 +46,7 @@
 
 static struct net_device *clip_devs;
 static struct atm_vcc *atmarpd;
+static DEFINE_MUTEX(atmarpd_lock);
 static struct timer_list idle_timer;
 static const struct neigh_ops clip_neigh_ops;
 
@@ -53,24 +54,35 @@ static int to_atmarpd(enum atmarp_ctrl_type type, int itf, __be32 ip)
 {
 	struct sock *sk;
 	struct atmarp_ctrl *ctrl;
+	struct atm_vcc *vcc;
 	struct sk_buff *skb;
+	int err = 0;
 
 	pr_debug("(%d)\n", type);
-	if (!atmarpd)
-		return -EUNATCH;
+
+	rcu_read_lock();
+	vcc = rcu_dereference(atmarpd);
+	if (!vcc) {
+		err = -EUNATCH;
+		goto unlock;
+	}
 	skb = alloc_skb(sizeof(struct atmarp_ctrl), GFP_ATOMIC);
-	if (!skb)
-		return -ENOMEM;
+	if (!skb) {
+		err = -ENOMEM;
+		goto unlock;
+	}
 	ctrl = skb_put(skb, sizeof(struct atmarp_ctrl));
 	ctrl->type = type;
 	ctrl->itf_num = itf;
 	ctrl->ip = ip;
-	atm_force_charge(atmarpd, skb->truesize);
+	atm_force_charge(vcc, skb->truesize);
 
-	sk = sk_atm(atmarpd);
+	sk = sk_atm(vcc);
 	skb_queue_tail(&sk->sk_receive_queue, skb);
 	sk->sk_data_ready(sk);
-	return 0;
+unlock:
+	rcu_read_unlock();
+	return err;
 }
 
 static void link_vcc(struct clip_vcc *clip_vcc, struct atmarp_entry *entry)
@@ -609,10 +621,12 @@ static void atmarpd_close(struct atm_vcc *vcc)
 {
 	pr_debug("\n");
 
-	rtnl_lock();
-	atmarpd = NULL;
+	mutex_lock(&atmarpd_lock);
+	RCU_INIT_POINTER(atmarpd, NULL);
+	mutex_unlock(&atmarpd_lock);
+
+	synchronize_rcu();
 	skb_queue_purge(&sk_atm(vcc)->sk_receive_queue);
-	rtnl_unlock();
 
 	pr_debug("(done)\n");
 	module_put(THIS_MODULE);
@@ -633,15 +647,15 @@ static struct atm_dev atmarpd_dev = {
 
 static int atm_init_atmarp(struct atm_vcc *vcc)
 {
-	rtnl_lock();
+	mutex_lock(&atmarpd_lock);
 	if (atmarpd) {
-		rtnl_unlock();
+		mutex_unlock(&atmarpd_lock);
 		return -EADDRINUSE;
 	}
 
 	mod_timer(&idle_timer, jiffies + CLIP_CHECK_INTERVAL * HZ);
 
-	atmarpd = vcc;
+	rcu_assign_pointer(atmarpd, vcc);
 	set_bit(ATM_VF_META, &vcc->flags);
 	set_bit(ATM_VF_READY, &vcc->flags);
 	    /* allow replies and avoid getting closed if signaling dies */
@@ -650,7 +664,7 @@ static int atm_init_atmarp(struct atm_vcc *vcc)
 	vcc->push = NULL;
 	vcc->pop = NULL; /* crash */
 	vcc->push_oam = NULL; /* crash */
-	rtnl_unlock();
+	mutex_unlock(&atmarpd_lock);
 	return 0;
 }
 
-- 
2.50.0.727.gbf7dc18ff4-goog

Re: [PATCH v1 net 1/2] atm: clip: Fix infinite recursive call of clip_push().

[Eric Dumazet]

On Tue, Jul 1, 2025 at 7:04 PM Kuniyuki Iwashima <kuniyu@google.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@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=2371d94d248d126c1eb1
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.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
>

Re: [PATCH v1 net 1/2] atm: clip: Fix infinite recursive call of clip_push().

[Kuniyuki Iwashima]

On Wed, Jul 2, 2025 at 1:03 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Tue, Jul 1, 2025 at 7:04 PM Kuniyuki Iwashima <kuniyu@google.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@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=2371d94d248d126c1eb1
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.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!

Re: [PATCH v1 net 2/2] atm: clip: Fix potential null-ptr-deref in to_atmarpd().

[kernel test robot]

Hi Kuniyuki,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Kuniyuki-Iwashima/atm-clip-Fix-infinite-recursive-call-of-clip_push/20250702-100652
base:   net/main
patch link:    https://lore.kernel.org/r/20250702020437.703698-3-kuniyu%40google.com
patch subject: [PATCH v1 net 2/2] atm: clip: Fix potential null-ptr-deref in to_atmarpd().
config: i386-randconfig-063-20250704 (https://download.01.org/0day-ci/archive/20250704/202507041113.tpHgxTvk-lkp@intel.com/config)
compiler: clang version 20.1.7 (https://github.com/llvm/llvm-project 6146a88f60492b520a36f8f8f3231e15f3cc6082)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250704/202507041113.tpHgxTvk-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507041113.tpHgxTvk-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> net/atm/clip.c:64:15: sparse: sparse: incompatible types in comparison expression (different address spaces):
   net/atm/clip.c:64:15: sparse:    struct atm_vcc [noderef] __rcu *
   net/atm/clip.c:64:15: sparse:    struct atm_vcc *
   net/atm/clip.c:625:9: sparse: sparse: incompatible types in comparison expression (different address spaces):
   net/atm/clip.c:625:9: sparse:    struct atm_vcc [noderef] __rcu *
   net/atm/clip.c:625:9: sparse:    struct atm_vcc *
   net/atm/clip.c:658:9: sparse: sparse: incompatible types in comparison expression (different address spaces):
   net/atm/clip.c:658:9: sparse:    struct atm_vcc [noderef] __rcu *
   net/atm/clip.c:658:9: sparse:    struct atm_vcc *

vim +64 net/atm/clip.c

    52	
    53	static int to_atmarpd(enum atmarp_ctrl_type type, int itf, __be32 ip)
    54	{
    55		struct sock *sk;
    56		struct atmarp_ctrl *ctrl;
    57		struct atm_vcc *vcc;
    58		struct sk_buff *skb;
    59		int err = 0;
    60	
    61		pr_debug("(%d)\n", type);
    62	
    63		rcu_read_lock();
  > 64		vcc = rcu_dereference(atmarpd);
    65		if (!vcc) {
    66			err = -EUNATCH;
    67			goto unlock;
    68		}
    69		skb = alloc_skb(sizeof(struct atmarp_ctrl), GFP_ATOMIC);
    70		if (!skb) {
    71			err = -ENOMEM;
    72			goto unlock;
    73		}
    74		ctrl = skb_put(skb, sizeof(struct atmarp_ctrl));
    75		ctrl->type = type;
    76		ctrl->itf_num = itf;
    77		ctrl->ip = ip;
    78		atm_force_charge(vcc, skb->truesize);
    79	
    80		sk = sk_atm(vcc);
    81		skb_queue_tail(&sk->sk_receive_queue, skb);
    82		sk->sk_data_ready(sk);
    83	unlock:
    84		rcu_read_unlock();
    85		return err;
    86	}
    87	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki