[PATCH bpf] bpf, sockmap: Fix NULL deref in sk_psock_backlog

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Xu Kuohai <xukuohai@xxxxxxxxxx>

sk_psock_backlog triggers a NULL dereference:

 BUG: kernel NULL pointer dereference, address: 000000000000000e
 #PF: supervisor read access in kernel mode
 #PF: error_code(0x0000) - not-present page
 PGD 0 P4D 0
 Oops: 0000 [#1] PREEMPT SMP PTI
 CPU: 0 PID: 70 Comm: kworker/0:3 Not tainted 6.5.0-rc2-00585-gb11bbbe4c66e #26
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b3f840-p4
 Workqueue: events sk_psock_backlog
 RIP: 0010:0xffffffffc0205254
 Code: 00 00 48 89 94 24 a0 00 00 00 41 5f 41 5e 41 5d 41 5c 5d 5b 41 5b 41 5a 41 59 41 50
 RSP: 0018:ffffc90000acbcb8 EFLAGS: 00010246
 RAX: ffffffff81c5ee10 RBX: ffff888018260000 RCX: 0000000000000001
 RDX: 0000000000000003 RSI: ffffc90000acbd58 RDI: 0000000000000000
 RBP: 0000000000000000 R08: 0000000000000003 R09: 0000000080100005
 R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000003
 R13: 0000000000000000 R14: 0000000000000021 R15: 0000000000000003
 FS:  0000000000000000(0000) GS:ffff88803ea00000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 000000000000000e CR3: 000000000b0de002 CR4: 0000000000170ef0
 Call Trace:
  <TASK>
  ? __die+0x24/0x70
  ? page_fault_oops+0x15d/0x480
  ? fixup_exception+0x26/0x330
  ? exc_page_fault+0x72/0x1d0
  ? asm_exc_page_fault+0x26/0x30
  ? __pfx_inet_sendmsg+0x10/0x10
  ? 0xffffffffc0205254
  ? inet_sendmsg+0x20/0x80
  ? sock_sendmsg+0x8f/0xa0
  ? __skb_send_sock+0x315/0x360
  ? __pfx_sendmsg_unlocked+0x10/0x10
  ? sk_psock_backlog+0xb4/0x300
  ? process_one_work+0x292/0x560
  ? worker_thread+0x53/0x3e0
  ? __pfx_worker_thread+0x10/0x10
  ? kthread+0x102/0x130
  ? __pfx_kthread+0x10/0x10
  ? ret_from_fork+0x34/0x50
  ? __pfx_kthread+0x10/0x10
  ? ret_from_fork_asm+0x1b/0x30
  </TASK>

The bug flow is as follows:

thread 1                                   thread 2

sk_psock_backlog                           sock_close
  sk_psock_handle_skb                        __sock_release
    __skb_send_sock                            inet_release
      sendmsg_unlocked                           tcp_close
        sock_sendmsg                               lock_sock
                                                     __tcp_close
                                                   release_sock
                                                 sock->sk = NULL // (1)
          inet_sendmsg
            sk = sock->sk // (2)
            inet_send_prepare
              inet_sk(sk)->inet_num // (3)

sock->sk is set to NULL by thread 2 at time (1), then fetched by
thread 1 at time (2), and used by thread 1 to access memory at
time (3), resulting in NULL pointer dereference.

To fix it, add lock_sock back on the egress path for sk_psock_handle_skb.

Fixes: 799aa7f98d53 ("skmsg: Avoid lock_sock() in sk_psock_backlog()")
Signed-off-by: Xu Kuohai <xukuohai@xxxxxxxxxx>
---
 net/core/skmsg.c | 44 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index 7c2764beeb04..8b758c51aa0d 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -609,15 +609,42 @@ static int sk_psock_skb_ingress_self(struct sk_psock *psock, struct sk_buff *skb
 	return err;
 }
 
+static int sk_psock_handle_ingress_skb(struct sk_psock *psock,
+				       struct sk_buff *skb,
+				       u32 off, u32 len)
+{
+	if (sock_flag(psock->sk, SOCK_DEAD))
+		return -EIO;
+	return sk_psock_skb_ingress(psock, skb, off, len);
+}
+
+static int sk_psock_handle_egress_skb(struct sk_psock *psock,
+				      struct sk_buff *skb,
+				      u32 off, u32 len)
+{
+	int ret;
+
+	lock_sock(psock->sk);
+
+	if (sock_flag(psock->sk, SOCK_DEAD))
+		ret = -EIO;
+	else if (!sock_writeable(psock->sk))
+		ret = -EAGAIN;
+	else
+		ret = skb_send_sock_locked(psock->sk, skb, off, len);
+
+	release_sock(psock->sk);
+
+	return ret;
+}
+
 static int sk_psock_handle_skb(struct sk_psock *psock, struct sk_buff *skb,
 			       u32 off, u32 len, bool ingress)
 {
-	if (!ingress) {
-		if (!sock_writeable(psock->sk))
-			return -EAGAIN;
-		return skb_send_sock(psock->sk, skb, off, len);
-	}
-	return sk_psock_skb_ingress(psock, skb, off, len);
+	if (ingress)
+		return sk_psock_handle_ingress_skb(psock, skb, off, len);
+	else
+		return sk_psock_handle_egress_skb(psock, skb, off, len);
 }
 
 static void sk_psock_skb_state(struct sk_psock *psock,
@@ -660,10 +687,7 @@ static void sk_psock_backlog(struct work_struct *work)
 		ingress = skb_bpf_ingress(skb);
 		skb_bpf_redirect_clear(skb);
 		do {
-			ret = -EIO;
-			if (!sock_flag(psock->sk, SOCK_DEAD))
-				ret = sk_psock_handle_skb(psock, skb, off,
-							  len, ingress);
+			ret = sk_psock_handle_skb(psock, skb, off, len, ingress);
 			if (ret <= 0) {
 				if (ret == -EAGAIN) {
 					sk_psock_skb_state(psock, state, len, off);
-- 
2.30.2





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux