[Patch net] atm: fix a UAF in lec_arp_clear_vccs()

[Patch net] atm: fix a UAF in lec_arp_clear_vccs()

[Cong Wang]

Gengming reported a UAF in lec_arp_clear_vccs(),
where we add a vcc socket to an entry in a per-device
list but free the socket without removing it from the
list when vcc->dev is NULL.

We need to call lec_vcc_close() to search and remove
those entries contain the vcc being destroyed. This can
be done by calling vcc->push(vcc, NULL) unconditionally
in vcc_destroy_socket().

Another issue discovered by Gengming's reproducer is
the vcc->dev may point to the static device lecatm_dev,
for which we don't need to register/unregister device,
so we can just check for vcc->dev->ops->owner.

Reported-by: Gengming Liu <l.dmxcsnsbh@gmail.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/atm/common.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/net/atm/common.c b/net/atm/common.c
index 0ce530af534d..8575f5d52087 100644
--- a/net/atm/common.c
+++ b/net/atm/common.c
@@ -177,18 +177,18 @@ static void vcc_destroy_socket(struct sock *sk)
 
 	set_bit(ATM_VF_CLOSE, &vcc->flags);
 	clear_bit(ATM_VF_READY, &vcc->flags);
-	if (vcc->dev) {
-		if (vcc->dev->ops->close)
-			vcc->dev->ops->close(vcc);
-		if (vcc->push)
-			vcc->push(vcc, NULL); /* atmarpd has no push */
-		module_put(vcc->owner);
-
-		while ((skb = skb_dequeue(&sk->sk_receive_queue)) != NULL) {
-			atm_return(vcc, skb->truesize);
-			kfree_skb(skb);
-		}
+	if (vcc->dev && vcc->dev->ops->close)
+		vcc->dev->ops->close(vcc);
+	if (vcc->push)
+		vcc->push(vcc, NULL); /* atmarpd has no push */
+	module_put(vcc->owner);
+
+	while ((skb = skb_dequeue(&sk->sk_receive_queue)) != NULL) {
+		atm_return(vcc, skb->truesize);
+		kfree_skb(skb);
+	}
 
+	if (vcc->dev && vcc->dev->ops->owner) {
 		module_put(vcc->dev->ops->owner);
 		atm_dev_put(vcc->dev);
 	}
-- 
2.26.1

[Patch net] atm: fix a memory leak of vcc->user_back

[Cong Wang]

In lec_arp_clear_vccs() only entry->vcc is freed, but vcc
could be installed on entry->recv_vcc too in lec_vcc_added().

This fixes the following memory leak:

unreferenced object 0xffff8880d9266b90 (size 16):
  comm "atm2", pid 425, jiffies 4294907980 (age 23.488s)
  hex dump (first 16 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 6b 6b 6b a5  ............kkk.
  backtrace:
    [<(____ptrval____)>] kmem_cache_alloc_trace+0x10e/0x151
    [<(____ptrval____)>] lane_ioctl+0x4b3/0x569
    [<(____ptrval____)>] do_vcc_ioctl+0x1ea/0x236
    [<(____ptrval____)>] svc_ioctl+0x17d/0x198
    [<(____ptrval____)>] sock_do_ioctl+0x47/0x12f
    [<(____ptrval____)>] sock_ioctl+0x2f9/0x322
    [<(____ptrval____)>] vfs_ioctl+0x1e/0x2b
    [<(____ptrval____)>] ksys_ioctl+0x61/0x80
    [<(____ptrval____)>] __x64_sys_ioctl+0x16/0x19
    [<(____ptrval____)>] do_syscall_64+0x57/0x65
    [<(____ptrval____)>] entry_SYSCALL_64_after_hwframe+0x49/0xb3

Cc: Gengming Liu <l.dmxcsnsbh@gmail.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/atm/lec.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index 25fa3a7b72bd..ca37f5a71f5e 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -1264,6 +1264,12 @@ static void lec_arp_clear_vccs(struct lec_arp_table *entry)
 		entry->vcc = NULL;
 	}
 	if (entry->recv_vcc) {
+		struct atm_vcc *vcc = entry->recv_vcc;
+		struct lec_vcc_priv *vpriv = LEC_VCC_PRIV(vcc);
+
+		kfree(vpriv);
+		vcc->user_back = NULL;
+
 		entry->recv_vcc->push = entry->old_recv_push;
 		vcc_release_async(entry->recv_vcc, -EPIPE);
 		entry->recv_vcc = NULL;
-- 
2.26.1

Re: [Patch net] atm: fix a UAF in lec_arp_clear_vccs()

[David Miller]

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Fri,  1 May 2020 11:11:08 -0700

> Gengming reported a UAF in lec_arp_clear_vccs(),
> where we add a vcc socket to an entry in a per-device
> list but free the socket without removing it from the
> list when vcc->dev is NULL.
> 
> We need to call lec_vcc_close() to search and remove
> those entries contain the vcc being destroyed. This can
> be done by calling vcc->push(vcc, NULL) unconditionally
> in vcc_destroy_socket().
> 
> Another issue discovered by Gengming's reproducer is
> the vcc->dev may point to the static device lecatm_dev,
> for which we don't need to register/unregister device,
> so we can just check for vcc->dev->ops->owner.
> 
> Reported-by: Gengming Liu <l.dmxcsnsbh@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Applied.

Re: [Patch net] atm: fix a memory leak of vcc->user_back

[David Miller]

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Fri,  1 May 2020 11:11:09 -0700

> In lec_arp_clear_vccs() only entry->vcc is freed, but vcc
> could be installed on entry->recv_vcc too in lec_vcc_added().
> 
> This fixes the following memory leak:
> 
> unreferenced object 0xffff8880d9266b90 (size 16):
>   comm "atm2", pid 425, jiffies 4294907980 (age 23.488s)
>   hex dump (first 16 bytes):
>     00 00 00 00 00 00 00 00 00 00 00 00 6b 6b 6b a5  ............kkk.
>   backtrace:
>     [<(____ptrval____)>] kmem_cache_alloc_trace+0x10e/0x151
>     [<(____ptrval____)>] lane_ioctl+0x4b3/0x569
>     [<(____ptrval____)>] do_vcc_ioctl+0x1ea/0x236
>     [<(____ptrval____)>] svc_ioctl+0x17d/0x198
>     [<(____ptrval____)>] sock_do_ioctl+0x47/0x12f
>     [<(____ptrval____)>] sock_ioctl+0x2f9/0x322
>     [<(____ptrval____)>] vfs_ioctl+0x1e/0x2b
>     [<(____ptrval____)>] ksys_ioctl+0x61/0x80
>     [<(____ptrval____)>] __x64_sys_ioctl+0x16/0x19
>     [<(____ptrval____)>] do_syscall_64+0x57/0x65
>     [<(____ptrval____)>] entry_SYSCALL_64_after_hwframe+0x49/0xb3
> 
> Cc: Gengming Liu <l.dmxcsnsbh@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Applied.