Message-ID: <61d4b9354cf9b_4679220874@john.notmuch>
Date:   Tue, 04 Jan 2022 13:16:37 -0800
From:   John Fastabend <john.fastabend@...il.com>
To:     syzbot <syzbot+bb73e71cf4b8fd376a4f@...kaller.appspotmail.com>,
        andrii@...nel.org, ast@...nel.org, bpf@...r.kernel.org,
        daniel@...earbox.net, davem@...emloft.net, hawk@...nel.org,
        jakub@...udflare.com, john.fastabend@...il.com, kafai@...com,
        kpsingh@...nel.org, kuba@...nel.org, linux-kernel@...r.kernel.org,
        lmb@...udflare.com, netdev@...r.kernel.org, songliubraving@...com,
        syzkaller-bugs@...glegroups.com, yhs@...com
Subject: RE: [syzbot] KASAN: vmalloc-out-of-bounds Read in bpf_prog_put

syzbot wrote:
> Hello,
> 
> syzbot found the following issue on:
> 
> HEAD commit:    9eaa88c7036e Merge tag 'libata-5.16-rc6' of git://git.kern..
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=10ed4143b00000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=10f3f669b8093e95
> dashboard link: https://syzkaller.appspot.com/bug?extid=bb73e71cf4b8fd376a4f
> compiler:       Debian clang version 11.0.1-2, GNU ld (GNU Binutils for Debian) 2.35.2
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=112d6ca5b00000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=17393549b00000
> 
> The issue was bisected to:
> 
> commit 38207a5e81230d6ffbdd51e5fa5681be5116dcae
> Author: John Fastabend <john.fastabend@...il.com>
> Date:   Fri Nov 19 18:14:17 2021 +0000
> 
>     bpf, sockmap: Attach map progs to psock early for feature probes
> 
> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=13532e85b00000
> final oops:     https://syzkaller.appspot.com/x/report.txt?x=10d32e85b00000
> console output: https://syzkaller.appspot.com/x/log.txt?x=17532e85b00000
> 
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+bb73e71cf4b8fd376a4f@...kaller.appspotmail.com
> Fixes: 38207a5e8123 ("bpf, sockmap: Attach map progs to psock early for feature probes")
> 
> ==================================================================
> BUG: KASAN: vmalloc-out-of-bounds in __bpf_prog_put kernel/bpf/syscall.c:1812 [inline]
> BUG: KASAN: vmalloc-out-of-bounds in __bpf_prog_put kernel/bpf/syscall.c:1812 [inline] kernel/bpf/syscall.c:1829
> BUG: KASAN: vmalloc-out-of-bounds in bpf_prog_put+0x8c/0x4f0 kernel/bpf/syscall.c:1829 kernel/bpf/syscall.c:1829
> Read of size 8 at addr ffffc90000e76038 by task syz-executor020/3641
> 
> CPU: 1 PID: 3641 Comm: syz-executor020 Not tainted 5.16.0-rc5-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
>  <TASK>
>  __dump_stack lib/dump_stack.c:88 [inline]
>  __dump_stack lib/dump_stack.c:88 [inline] lib/dump_stack.c:106
>  dump_stack_lvl+0x1dc/0x2d8 lib/dump_stack.c:106 lib/dump_stack.c:106
>  print_address_description+0x65/0x380 mm/kasan/report.c:247 mm/kasan/report.c:247
>  __kasan_report mm/kasan/report.c:433 [inline]
>  __kasan_report mm/kasan/report.c:433 [inline] mm/kasan/report.c:450
>  kasan_report+0x19a/0x1f0 mm/kasan/report.c:450 mm/kasan/report.c:450
>  __bpf_prog_put kernel/bpf/syscall.c:1812 [inline]
>  __bpf_prog_put kernel/bpf/syscall.c:1812 [inline] kernel/bpf/syscall.c:1829
>  bpf_prog_put+0x8c/0x4f0 kernel/bpf/syscall.c:1829 kernel/bpf/syscall.c:1829
>  bpf_prog_release+0x37/0x40 kernel/bpf/syscall.c:1837 kernel/bpf/syscall.c:1837
>  __fput+0x3fc/0x870 fs/file_table.c:280 fs/file_table.c:280
>  task_work_run+0x146/0x1c0 kernel/task_work.c:164 kernel/task_work.c:164
>  exit_task_work include/linux/task_work.h:32 [inline]
>  exit_task_work include/linux/task_work.h:32 [inline] kernel/exit.c:832
>  do_exit+0x705/0x24f0 kernel/exit.c:832 kernel/exit.c:832
>  do_group_exit+0x168/0x2d0 kernel/exit.c:929 kernel/exit.c:929
>  __do_sys_exit_group+0x13/0x20 kernel/exit.c:940 kernel/exit.c:940
>  __se_sys_exit_group+0x10/0x10 kernel/exit.c:938 kernel/exit.c:938
>  __x64_sys_exit_group+0x37/0x40 kernel/exit.c:938 kernel/exit.c:938
>  do_syscall_x64 arch/x86/entry/common.c:50 [inline]
>  do_syscall_x64 arch/x86/entry/common.c:50 [inline] arch/x86/entry/common.c:80
>  do_syscall_64+0x44/0xd0 arch/x86/entry/common.c:80 arch/x86/entry/common.c:80
>  entry_SYSCALL_64_after_hwframe+0x44/0xae
> RIP: 0033:0x7f3b90ccd1d9
> Code: Unable to access opcode bytes at RIP 0x7f3b90ccd1af.
> RSP: 002b:00007ffdeec58318 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
> RAX: ffffffffffffffda RBX: 00007f3b90d41330 RCX: 00007f3b90ccd1d9
> RDX: 000000000000003c RSI: 00000000000000e7 RDI: 0000000000000000
> RBP: 0000000000000000 R08: ffffffffffffffc4 R09: 00007ffdeec58390
> R10: 00007ffdeec58390 R11: 0000000000000246 R12: 00007f3b90d41330
> R13: 0000000000000001 R14: 0000000000000000 R15: 0000000000000001
>  </TASK>
> 
> 
> Memory state around the buggy address:
>  ffffc90000e75f00: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
>  ffffc90000e75f80: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> >ffffc90000e76000: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
>                                         ^
>  ffffc90000e76080: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
>  ffffc90000e76100: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> ==================================================================
> 
> 
> ---
> This report is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@...glegroups.com.
> 
> syzbot will keep track of this issue. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
> For information about bisection process see: https://goo.gl/tpsmEJ#bisection
> syzbot can test patches for this issue, for details see:
> https://goo.gl/tpsmEJ#testing-patches

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master

>From f072b6529a5d41e9af8ed04d9f31108a0baa2eec Mon Sep 17 00:00:00 2001
From: John Fastabend <john.fastabend@...il.com>
Date: Tue, 4 Jan 2022 11:10:38 -0800
Subject: [PATCH bpf-next] bpf, sockmap: fix double bpf_prog_put on error case
 in map_link

sock_map_link() is called to update a sockmap entry with a sk. But, if the
sock_map_init_proto() call fails then we return an error to the map_update
op against the sockmap. In the error path though we need to cleanup psock
and dec the refcnt on any programs associated with the map, because we
refcnt them early in the update process to ensure they are pinned for the
psock. (This avoids a race where user deletes programs while also updating
the map with new socks.)

In current code we do the prog refcnt dec explicitely by calling
bpf_prog_put() when the program was found in the map. But, after commit
'38207a5e81230' in this error path we've already done the prog to psock
assignment so the programs have a reference from the psock as well. This
then causes the psock tear down logic, invoked by sk_psock_put() in the
error path, to similarly call bpf_prog_put on the programs there.

To be explicit this logic does the prog->psock assignemnt

  if (msg_*)
    psock_set_prog(...)

Then the error path under the out_progs label does a similar check and dec
with,

  if (msg_*)
     bpf_prog_put(...)

And the teardown logic sk_psock_put() does,

  psock_set_prog(msg_*, NULL)

triggering another bpf_prog_put(...). Then KASAN gives us this splat, found
by syzbot because we've created an inbalance between bpf_prog_inc and
bpf_prog_put calling put twice on the program.

BUG: KASAN: vmalloc-out-of-bounds in __bpf_prog_put kernel/bpf/syscall.c:1812 [inline]
BUG: KASAN: vmalloc-out-of-bounds in __bpf_prog_put kernel/bpf/syscall.c:1812 [inline] kernel/bpf/syscall.c:1829
BUG: KASAN: vmalloc-out-of-bounds in bpf_prog_put+0x8c/0x4f0 kernel/bpf/syscall.c:1829 kernel/bpf/syscall.c:1829
Read of size 8 at addr ffffc90000e76038 by task syz-executor020/3641

To fix clean up error path so it doesn't try to do the bpf_prog_put in the
error path once progs are assigned then it relies on the normal psock
tear down logic to do complete cleanup.

For completness we also cover the case whereh sk_psock_init_strp() fails,
but this is not expected because it indicates an incorrect socket type
and should be caught earlier.

Reported-by: syzbot+bb73e71cf4b8fd376a4f@...kaller.appspotmail.com
Fixes: 38207a5e8123 ("bpf, sockmap: Attach map progs to psock early for feature probes")
Signed-off-by: John Fastabend <john.fastabend@...il.com>
---
 net/core/sock_map.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index 9618ab6d7cc9..1827669eedd6 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -292,15 +292,23 @@ static int sock_map_link(struct bpf_map *map, struct sock *sk)
 	if (skb_verdict)
 		psock_set_prog(&psock->progs.skb_verdict, skb_verdict);
 
+	/* msg_* and stream_* programs references tracked in psock after this
+	 * point. Reference dec and cleanup will occur through psock destructor
+	 */
 	ret = sock_map_init_proto(sk, psock);
-	if (ret < 0)
-		goto out_drop;
+	if (ret < 0) {
+		sk_psock_put(sk, psock);
+		goto out;
+	}
 
 	write_lock_bh(&sk->sk_callback_lock);
 	if (stream_parser && stream_verdict && !psock->saved_data_ready) {
 		ret = sk_psock_init_strp(sk, psock);
-		if (ret)
-			goto out_unlock_drop;
+		if (ret) {
+			write_unlock_bh(&sk->sk_callback_lock);
+			sk_psock_put(sk, psock);
+			goto out;
+		}
 		sk_psock_start_strp(sk, psock);
 	} else if (!stream_parser && stream_verdict && !psock->saved_data_ready) {
 		sk_psock_start_verdict(sk,psock);
@@ -309,10 +317,6 @@ static int sock_map_link(struct bpf_map *map, struct sock *sk)
 	}
 	write_unlock_bh(&sk->sk_callback_lock);
 	return 0;
-out_unlock_drop:
-	write_unlock_bh(&sk->sk_callback_lock);
-out_drop:
-	sk_psock_put(sk, psock);
 out_progs:
 	if (skb_verdict)
 		bpf_prog_put(skb_verdict);
@@ -325,6 +329,7 @@ static int sock_map_link(struct bpf_map *map, struct sock *sk)
 out_put_stream_verdict:
 	if (stream_verdict)
 		bpf_prog_put(stream_verdict);
+out:
 	return ret;
 }
 
-- 
2.33.0

Powered by blists - more mailing lists