[PATCH v2 net 0/3] atm: clip: Fix infinite recursion, potential null-ptr-deref, and memleak.

[PATCH v2 net 0/3] atm: clip: Fix infinite recursion, potential null-ptr-deref, and memleak.

[Kuniyuki Iwashima]

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

Patch 2 fixes memory leak by ioctl(ATMARP_MKIP) and ioctl(ATMARPD_CTRL).

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


Changes:
  v2:
    * Add patch 2
    * Patch 1: Silence Sparse __rcu warning

  v1: https://lore.kernel.org/netdev/20250702020437.703698-1-kuniyu@google.com/


Kuniyuki Iwashima (3):
  atm: clip: Fix potential null-ptr-deref in to_atmarpd().
  atm: clip: Fix memory leak of struct clip_vcc.
  atm: clip: Fix infinite recursive call of clip_push().

 net/atm/clip.c | 54 ++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 39 insertions(+), 15 deletions(-)

-- 
2.50.0.727.gbf7dc18ff4-goog

[PATCH v2 net 1/3] 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>
---
v2: Add __rcu to silence Sparse
---
 net/atm/clip.c | 44 +++++++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/net/atm/clip.c b/net/atm/clip.c
index b234dc3bcb0d..f36f2c7d8714 100644
--- a/net/atm/clip.c
+++ b/net/atm/clip.c
@@ -45,7 +45,8 @@
 #include <net/atmclip.h>
 
 static struct net_device *clip_devs;
-static struct atm_vcc *atmarpd;
+static struct atm_vcc __rcu *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)
@@ -607,10 +619,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);
@@ -631,15 +645,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 */
@@ -648,7 +662,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

[PATCH v2 net 2/3] atm: clip: Fix memory leak of struct clip_vcc.

[Kuniyuki Iwashima]

ioctl(ATMARP_MKIP) allocates struct clip_vcc and set it to
vcc->user_back.

The code assumes that vcc_destroy_socket() passes NULL skb
to vcc->push() when the socket is close()d, and then clip_push()
frees clip_vcc.

However, ioctl(ATMARPD_CTRL) sets NULL to vcc->push() in
atm_init_atmarp(), resulting in memory leak.

Let's serialise two ioctl() by lock_sock() and check vcc->push()
in atm_init_atmarp() to prevent memleak.

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

diff --git a/net/atm/clip.c b/net/atm/clip.c
index f36f2c7d8714..9c9c6c3d9886 100644
--- a/net/atm/clip.c
+++ b/net/atm/clip.c
@@ -645,6 +645,9 @@ static struct atm_dev atmarpd_dev = {
 
 static int atm_init_atmarp(struct atm_vcc *vcc)
 {
+	if (vcc->push == clip_push)
+		return -EINVAL;
+
 	mutex_lock(&atmarpd_lock);
 	if (atmarpd) {
 		mutex_unlock(&atmarpd_lock);
@@ -669,6 +672,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) {
@@ -689,14 +693,18 @@ static int clip_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 		err = clip_create(arg);
 		break;
 	case ATMARPD_CTRL:
+		lock_sock(sk);
 		err = atm_init_atmarp(vcc);
 		if (!err) {
 			sock->state = SS_CONNECTED;
 			__module_get(THIS_MODULE);
 		}
+		release_sock(sk);
 		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 v2 net 3/3] 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 the socket is close()d, vcc_destroy_socket() passes
NULL skb to clip_push(), which 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 | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/atm/clip.c b/net/atm/clip.c
index 9c9c6c3d9886..a30c5a270545 100644
--- a/net/atm/clip.c
+++ b/net/atm/clip.c
@@ -429,6 +429,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;
-- 
2.50.0.727.gbf7dc18ff4-goog

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

[Simon Horman]

On Fri, Jul 04, 2025 at 06:23:51AM +0000, Kuniyuki Iwashima wrote:
> 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>
> ---
> v2: Add __rcu to silence Sparse

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH v2 net 2/3] atm: clip: Fix memory leak of struct clip_vcc.

[Simon Horman]

On Fri, Jul 04, 2025 at 06:23:52AM +0000, Kuniyuki Iwashima wrote:
> ioctl(ATMARP_MKIP) allocates struct clip_vcc and set it to
> vcc->user_back.
> 
> The code assumes that vcc_destroy_socket() passes NULL skb
> to vcc->push() when the socket is close()d, and then clip_push()
> frees clip_vcc.
> 
> However, ioctl(ATMARPD_CTRL) sets NULL to vcc->push() in
> atm_init_atmarp(), resulting in memory leak.
> 
> Let's serialise two ioctl() by lock_sock() and check vcc->push()
> in atm_init_atmarp() to prevent memleak.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH v2 net 3/3] atm: clip: Fix infinite recursive call of clip_push().

[Simon Horman]

On Fri, Jul 04, 2025 at 06:23:53AM +0000, Kuniyuki Iwashima 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 the socket is close()d, vcc_destroy_socket() passes
> NULL skb to clip_push(), which 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>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH v2 net 0/3] atm: clip: Fix infinite recursion, potential null-ptr-deref, and memleak.

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri,  4 Jul 2025 06:23:50 +0000 you wrote:
> Patch 1 fixes racy access to atmarpd found while checking RTNL usage
> in clip.c.
> 
> Patch 2 fixes memory leak by ioctl(ATMARP_MKIP) and ioctl(ATMARPD_CTRL).
> 
> Patch 3 fixes infinite recursive call of clip_vcc->old_push(), which
> was reported by syzbot.
> 
> [...]

Here is the summary with links:
  - [v2,net,1/3] atm: clip: Fix potential null-ptr-deref in to_atmarpd().
    https://git.kernel.org/netdev/net/c/706cc3647713
  - [v2,net,2/3] atm: clip: Fix memory leak of struct clip_vcc.
    https://git.kernel.org/netdev/net/c/62dba28275a9
  - [v2,net,3/3] atm: clip: Fix infinite recursive call of clip_push().
    https://git.kernel.org/netdev/net/c/c489f3283dbf

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html