linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* WARNING in __vunmap
@ 2019-02-16  7:01 syzbot
  2019-09-08  7:05 ` syzbot
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: syzbot @ 2019-02-16  7:01 UTC (permalink / raw)
  To: davem, herbert, linux-kernel, netdev, steffen.klassert, syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    a048a07d7f45 powerpc/64s: Add support for a store forwardi..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1418d5a7800000
kernel config:  https://syzkaller.appspot.com/x/.config?x=982e2df1b9e60b02
dashboard link: https://syzkaller.appspot.com/bug?extid=5ec9bb042ddfe9644773
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)

Unfortunately, I don't have any reproducer for this crash yet.

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+5ec9bb042ddfe9644773@syzkaller.appspotmail.com

------------[ cut here ]------------
Trying to vfree() nonexistent vm area (0000000094b55709)
WARNING: CPU: 1 PID: 8605 at mm/vmalloc.c:1525 __vunmap+0x324/0x3c0  
mm/vmalloc.c:1524
Kernel panic - not syncing: panic_on_warn set ...

CPU: 1 PID: 8605 Comm: syz-executor7 Not tainted 4.17.0-rc6+ #63
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x1b9/0x294 lib/dump_stack.c:113
  panic+0x22f/0x4de kernel/panic.c:184
  __warn.cold.8+0x163/0x1b3 kernel/panic.c:536
  report_bug+0x252/0x2d0 lib/bug.c:186
  fixup_bug arch/x86/kernel/traps.c:178 [inline]
  do_error_trap+0x1de/0x490 arch/x86/kernel/traps.c:296
  do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:315
  invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:992
RIP: 0010:__vunmap+0x324/0x3c0 mm/vmalloc.c:1524
RSP: 0018:ffff88018273f2b0 EFLAGS: 00010286
RAX: 0000000000000038 RBX: 0000000000000000 RCX: ffffc90003ed8000
RDX: 0000000000003b55 RSI: ffffffff81610961 RDI: ffff88018273ee10
RBP: ffff88018273f2f0 R08: ffff8801ab1a0340 R09: 0000000000000006
R10: ffff8801ab1a0340 R11: 0000000000000000 R12: ffffc90014122000
R13: 0000000000000001 R14: fffffbfff12f7f76 R15: 000060fe24e7fae8
  vfree+0x68/0x100 mm/vmalloc.c:1606
  ipcomp_free_scratches+0xbb/0x150 net/xfrm/xfrm_ipcomp.c:216
  ipcomp_free_data net/xfrm/xfrm_ipcomp.c:325 [inline]
  ipcomp_init_state+0x8cf/0xc00 net/xfrm/xfrm_ipcomp.c:377
  ipcomp4_init_state+0x110/0xb30 net/ipv4/ipcomp.c:137
  __xfrm_init_state+0x60e/0x1030 net/xfrm/xfrm_state.c:2277
  xfrm_init_state+0x1e/0x80 net/xfrm/xfrm_state.c:2303
  pfkey_msg2xfrm_state net/key/af_key.c:1307 [inline]
  pfkey_add+0x1c86/0x2eb0 net/key/af_key.c:1524
  pfkey_process+0x83d/0x980 net/key/af_key.c:2844
  pfkey_sendmsg+0x5f4/0x1050 net/key/af_key.c:3683
  sock_sendmsg_nosec net/socket.c:629 [inline]
  sock_sendmsg+0xd5/0x120 net/socket.c:639
  ___sys_sendmsg+0x805/0x940 net/socket.c:2117
  __sys_sendmsg+0x115/0x270 net/socket.c:2155
  __do_sys_sendmsg net/socket.c:2164 [inline]
  __se_sys_sendmsg net/socket.c:2162 [inline]
  __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2162
  do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x455a09
RSP: 002b:00007fe6fd709c68 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00007fe6fd70a6d4 RCX: 0000000000455a09
RDX: 0000000000000000 RSI: 0000000020f56000 RDI: 0000000000000014
RBP: 000000000072bea0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 000000000000059d R14: 00000000006fc758 R15: 0000000000000000
Dumping ftrace buffer:
    (ftrace buffer empty)
Kernel Offset: disabled
Rebooting in 86400 seconds..


---
This bug 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@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with  
syzbot.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: WARNING in __vunmap
  2019-02-16  7:01 WARNING in __vunmap syzbot
@ 2019-09-08  7:05 ` syzbot
  2021-08-04  5:35 ` [syzbot] " syzbot
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: syzbot @ 2019-09-08  7:05 UTC (permalink / raw)
  To: davem, herbert, linux-kernel, netdev, steffen.klassert, syzkaller-bugs

syzbot has found a reproducer for the following crash on:

HEAD commit:    b3a9964c Merge tag 'char-misc-5.3-rc8' of git://git.kernel..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=16c9f70a600000
kernel config:  https://syzkaller.appspot.com/x/.config?x=144488c6c6c6d2b6
dashboard link: https://syzkaller.appspot.com/bug?extid=5ec9bb042ddfe9644773
compiler:       clang version 9.0.0 (/home/glider/llvm/clang  
80fee25776c2fb61e74c1ecb1a523375c2500b69)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=11a30371600000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+5ec9bb042ddfe9644773@syzkaller.appspotmail.com

------------[ cut here ]------------
Trying to vfree() nonexistent vm area (00000000dddfa71b)
WARNING: CPU: 0 PID: 10463 at mm/vmalloc.c:2235 __vunmap+0x148/0xa20  
mm/vmalloc.c:2234
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 10463 Comm: syz-executor.0 Not tainted 5.3.0-rc7+ #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x1d8/0x2f8 lib/dump_stack.c:113
  panic+0x25c/0x799 kernel/panic.c:219
  __warn+0x22f/0x230 kernel/panic.c:576
  report_bug+0x190/0x290 lib/bug.c:186
  fixup_bug arch/x86/kernel/traps.c:179 [inline]
  do_error_trap+0xd7/0x440 arch/x86/kernel/traps.c:272
  do_invalid_op+0x36/0x40 arch/x86/kernel/traps.c:291
  invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1028
RIP: 0010:__vunmap+0x148/0xa20 mm/vmalloc.c:2234
Code: 0c e8 8c 36 d0 ff eb 24 e8 85 36 d0 ff 48 c7 c7 a8 f5 8f 88 e8 69 9b  
d8 05 48 c7 c7 e5 5c 3e 88 4c 89 f6 31 c0 e8 68 18 a3 ff <0f> 0b 48 83 c4  
60 5b 41 5c 41 5d 41 5e 41 5f 5d c3 48 c7 c7 a8 f5
RSP: 0018:ffff8880a81d75b8 EFLAGS: 00010246
RAX: 4324ba28a2c9f400 RBX: 0000000000000000 RCX: ffff8880a48ce080
RDX: 0000000000000000 RSI: 0000000080000000 RDI: 0000000000000000
RBP: ffff8880a81d7640 R08: ffffffff815cfa54 R09: ffffed1015d46088
R10: ffffed1015d46088 R11: 0000000000000000 R12: ffff88808a93f708
R13: dffffc0000000000 R14: ffffc900080f7000 R15: ffffc90008108000
  __vfree mm/vmalloc.c:2299 [inline]
  vfree+0x85/0x130 mm/vmalloc.c:2329
  ipcomp_free_scratches net/xfrm/xfrm_ipcomp.c:212 [inline]
  ipcomp_free_data+0x12a/0x1d0 net/xfrm/xfrm_ipcomp.c:321
  ipcomp_init_state+0x7bf/0x8b0 net/xfrm/xfrm_ipcomp.c:373
  ipcomp6_init_state+0xb7/0x630 net/ipv6/ipcomp6.c:153
  __xfrm_init_state+0x7d0/0xbf0 net/xfrm/xfrm_state.c:2493
  xfrm_state_construct net/xfrm/xfrm_user.c:626 [inline]
  xfrm_add_sa+0x223f/0x38e0 net/xfrm/xfrm_user.c:683
  xfrm_user_rcv_msg+0x3e6/0x650 net/xfrm/xfrm_user.c:2676
  netlink_rcv_skb+0x19e/0x3d0 net/netlink/af_netlink.c:2477
  xfrm_netlink_rcv+0x74/0x90 net/xfrm/xfrm_user.c:2684
  netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline]
  netlink_unicast+0x787/0x900 net/netlink/af_netlink.c:1328
  netlink_sendmsg+0x993/0xc50 net/netlink/af_netlink.c:1917
  sock_sendmsg_nosec net/socket.c:637 [inline]
  sock_sendmsg net/socket.c:657 [inline]
  ___sys_sendmsg+0x60d/0x910 net/socket.c:2311
  __sys_sendmsg net/socket.c:2356 [inline]
  __do_sys_sendmsg net/socket.c:2365 [inline]
  __se_sys_sendmsg net/socket.c:2363 [inline]
  __x64_sys_sendmsg+0x17c/0x200 net/socket.c:2363
  do_syscall_64+0xfe/0x140 arch/x86/entry/common.c:296
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4598e9
Code: fd b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 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 0f 83 cb b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f4880699c78 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000004598e9
RDX: 0000000000000000 RSI: 0000000020000840 RDI: 0000000000000003
RBP: 000000000075bfc8 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007f488069a6d4
R13: 00000000004c7812 R14: 00000000004dd0b0 R15: 00000000ffffffff
Kernel Offset: disabled
Rebooting in 86400 seconds..


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [syzbot] WARNING in __vunmap
  2019-02-16  7:01 WARNING in __vunmap syzbot
  2019-09-08  7:05 ` syzbot
@ 2021-08-04  5:35 ` syzbot
  2022-08-31  1:41 ` [PATCH] xfrm: Don't increase scratch users if allocation fails Khalid Masum
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: syzbot @ 2021-08-04  5:35 UTC (permalink / raw)
  To: ali.hamid, davem, hdanton, herbert, kuba, linux-kernel, netdev,
	steffen.klassert, syzkaller-bugs, xiyou.wangcong

syzbot has found a reproducer for the following issue on:

HEAD commit:    d5ad8ec3cfb5 Merge tag 'media/v5.14-2' of git://git.kernel..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1669619a300000
kernel config:  https://syzkaller.appspot.com/x/.config?x=343fd21f6f4da2d6
dashboard link: https://syzkaller.appspot.com/bug?extid=5ec9bb042ddfe9644773
compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.1
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=11c3f142300000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=154e2121300000

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+5ec9bb042ddfe9644773@syzkaller.appspotmail.com

------------[ cut here ]------------
Trying to vfree() nonexistent vm area (ffffc90002bc9000)
WARNING: CPU: 0 PID: 8497 at mm/vmalloc.c:2567 __vunmap+0x150/0xb70 mm/vmalloc.c:2567
Modules linked in:
CPU: 1 PID: 8497 Comm: syz-executor174 Not tainted 5.14.0-rc4-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:__vunmap+0x150/0xb70 mm/vmalloc.c:2567
Code: 85 78 ff ff ff e8 20 b0 c4 ff 48 c7 c7 c0 7c a9 8b e8 44 ed 7b 07 e8 0f b0 c4 ff 4c 89 e6 48 c7 c7 e0 bb 96 89 e8 c1 05 37 07 <0f> 0b 48 83 c4 38 5b 5d 41 5c 41 5d 41 5e 41 5f e9 eb af c4 ff e8
RSP: 0018:ffffc900023b72d8 EFLAGS: 00010286
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: ffff888028c00000 RSI: ffffffff815d7935 RDI: fffff52000476e4d
RBP: dffffc0000000000 R08: 0000000000000000 R09: 0000000000000000
R10: ffffffff815d176e R11: 0000000000000000 R12: ffffc90002bc9000
R13: ffff8880253d20c0 R14: ffffc90002bc9000 R15: ffffe8ffffc338a8
FS:  00007fcdcc063700(0000) GS:ffff8880b9c00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fcdcc084718 CR3: 00000000159f1000 CR4: 00000000001506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 __vfree+0x3c/0xd0 mm/vmalloc.c:2635
 vfree+0x5a/0x90 mm/vmalloc.c:2666
 ipcomp_free_scratches+0xc4/0x160 net/xfrm/xfrm_ipcomp.c:203
 ipcomp_free_data net/xfrm/xfrm_ipcomp.c:312 [inline]
 ipcomp_init_state+0x77c/0xa40 net/xfrm/xfrm_ipcomp.c:364
 ipcomp6_init_state+0xc2/0x700 net/ipv6/ipcomp6.c:154
 __xfrm_init_state+0x995/0x15c0 net/xfrm/xfrm_state.c:2648
 xfrm_state_construct net/xfrm/xfrm_user.c:627 [inline]
 xfrm_add_sa+0x1ef1/0x35f0 net/xfrm/xfrm_user.c:684
 xfrm_user_rcv_msg+0x42c/0x8b0 net/xfrm/xfrm_user.c:2812
 netlink_rcv_skb+0x153/0x420 net/netlink/af_netlink.c:2504
 xfrm_netlink_rcv+0x6b/0x90 net/xfrm/xfrm_user.c:2824
 netlink_unicast_kernel net/netlink/af_netlink.c:1314 [inline]
 netlink_unicast+0x533/0x7d0 net/netlink/af_netlink.c:1340
 netlink_sendmsg+0x86d/0xdb0 net/netlink/af_netlink.c:1929
 sock_sendmsg_nosec net/socket.c:703 [inline]
 sock_sendmsg+0xcf/0x120 net/socket.c:723
 ____sys_sendmsg+0x6e8/0x810 net/socket.c:2392
 ___sys_sendmsg+0xf3/0x170 net/socket.c:2446
 __sys_sendmsg+0xe5/0x1b0 net/socket.c:2475
 do_syscall_x64 arch/x86/entry/common.c:50 [inline]
 do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
 entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x445b99
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 11 15 00 00 90 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 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fcdcc063318 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00000000004ca428 RCX: 0000000000445b99
RDX: 0000000000000000 RSI: 0000000020000800 RDI: 0000000000000004
RBP: 00000000004ca420 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000004ca42c
R13: 00007ffec83642cf R14: 00007fcdcc063400 R15: 0000000000022000


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH] xfrm: Don't increase scratch users if allocation fails
  2019-02-16  7:01 WARNING in __vunmap syzbot
  2019-09-08  7:05 ` syzbot
  2021-08-04  5:35 ` [syzbot] " syzbot
@ 2022-08-31  1:41 ` Khalid Masum
  2022-08-31  9:13   ` Herbert Xu
  2022-08-31 14:29 ` [PATCH v2] xfrm: ipcomp: Update ipcomp_scratches with NULL if alloc fails Khalid Masum
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Khalid Masum @ 2022-08-31  1:41 UTC (permalink / raw)
  To: netdev, linux-kernel, syzkaller-bugs, syzbot+5ec9bb042ddfe9644773
  Cc: Steffen Klassert, Herbert Xu, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel-mentees, Shuah Khan,
	Khalid Masum

ipcomp_alloc_scratches() routine increases ipcomp_scratch_users count
even if it fails to allocate memory. Therefore, ipcomp_free_scratches()
routine, when triggered, tries to vfree() non existent percpu 
ipcomp_scratches.

To fix this breakage, do not increase scratch users count if
ipcomp_alloc_scratches() fails to allocate scratches.

Reported-and-tested-by: syzbot+5ec9bb042ddfe9644773@syzkaller.appspotmail.com
Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>

---
 net/xfrm/xfrm_ipcomp.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c
index cb40ff0ff28d..af9097983139 100644
--- a/net/xfrm/xfrm_ipcomp.c
+++ b/net/xfrm/xfrm_ipcomp.c
@@ -210,13 +210,15 @@ static void * __percpu *ipcomp_alloc_scratches(void)
 	void * __percpu *scratches;
 	int i;
 
-	if (ipcomp_scratch_users++)
+	if (ipcomp_scratch_users) {
+		ipcomp_scratch_users++;
 		return ipcomp_scratches;
-
+	}
 	scratches = alloc_percpu(void *);
 	if (!scratches)
 		return NULL;
 
+	ipcomp_scratch_users++;
 	ipcomp_scratches = scratches;
 
 	for_each_possible_cpu(i) {
-- 
2.37.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] xfrm: Don't increase scratch users if allocation fails
  2022-08-31  1:41 ` [PATCH] xfrm: Don't increase scratch users if allocation fails Khalid Masum
@ 2022-08-31  9:13   ` Herbert Xu
  2022-08-31 12:01     ` Khalid Masum
  0 siblings, 1 reply; 13+ messages in thread
From: Herbert Xu @ 2022-08-31  9:13 UTC (permalink / raw)
  To: Khalid Masum
  Cc: netdev, linux-kernel, syzkaller-bugs,
	syzbot+5ec9bb042ddfe9644773, Steffen Klassert, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel-mentees,
	Shuah Khan

On Wed, Aug 31, 2022 at 07:41:26AM +0600, Khalid Masum wrote:
> ipcomp_alloc_scratches() routine increases ipcomp_scratch_users count
> even if it fails to allocate memory. Therefore, ipcomp_free_scratches()
> routine, when triggered, tries to vfree() non existent percpu 
> ipcomp_scratches.
> 
> To fix this breakage, do not increase scratch users count if
> ipcomp_alloc_scratches() fails to allocate scratches.
> 
> Reported-and-tested-by: syzbot+5ec9bb042ddfe9644773@syzkaller.appspotmail.com
> Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>
> ---
>  net/xfrm/xfrm_ipcomp.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c
> index cb40ff0ff28d..af9097983139 100644
> --- a/net/xfrm/xfrm_ipcomp.c
> +++ b/net/xfrm/xfrm_ipcomp.c
> @@ -210,13 +210,15 @@ static void * __percpu *ipcomp_alloc_scratches(void)
>  	void * __percpu *scratches;
>  	int i;
>  
> -	if (ipcomp_scratch_users++)
> +	if (ipcomp_scratch_users) {
> +		ipcomp_scratch_users++;
>  		return ipcomp_scratches;
> -
> +	}
>  	scratches = alloc_percpu(void *);
>  	if (!scratches)
>  		return NULL;
>  
> +	ipcomp_scratch_users++;
>  	ipcomp_scratches = scratches;

This patch is broken because on error we will always call
ipcomp_free_scratches which frees any partially allocated memory
and restores ipcomp_scratch_users to zero.

With this patch ipcomp_scratch_users will turn negative on error.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] xfrm: Don't increase scratch users if allocation fails
  2022-08-31  9:13   ` Herbert Xu
@ 2022-08-31 12:01     ` Khalid Masum
  0 siblings, 0 replies; 13+ messages in thread
From: Khalid Masum @ 2022-08-31 12:01 UTC (permalink / raw)
  To: Herbert Xu
  Cc: netdev, linux-kernel, syzkaller-bugs,
	syzbot+5ec9bb042ddfe9644773, Steffen Klassert, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel-mentees,
	Shuah Khan

On 8/31/22 15:13, Herbert Xu wrote:
> On Wed, Aug 31, 2022 at 07:41:26AM +0600, Khalid Masum wrote:
>> ipcomp_alloc_scratches() routine increases ipcomp_scratch_users count
>> even if it fails to allocate memory. Therefore, ipcomp_free_scratches()
>> routine, when triggered, tries to vfree() non existent percpu
>> ipcomp_scratches.
>>
>> To fix this breakage, do not increase scratch users count if
>> ipcomp_alloc_scratches() fails to allocate scratches.
>>
>> Reported-and-tested-by: syzbot+5ec9bb042ddfe9644773@syzkaller.appspotmail.com
>> Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>
>> ---
>>   net/xfrm/xfrm_ipcomp.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c
>> index cb40ff0ff28d..af9097983139 100644
>> --- a/net/xfrm/xfrm_ipcomp.c
>> +++ b/net/xfrm/xfrm_ipcomp.c
>> @@ -210,13 +210,15 @@ static void * __percpu *ipcomp_alloc_scratches(void)
>>   	void * __percpu *scratches;
>>   	int i;
>>   
>> -	if (ipcomp_scratch_users++)
>> +	if (ipcomp_scratch_users) {
>> +		ipcomp_scratch_users++;
>>   		return ipcomp_scratches;
>> -
>> +	}
>>   	scratches = alloc_percpu(void *);
>>   	if (!scratches)
>>   		return NULL;
>>   
>> +	ipcomp_scratch_users++;
>>   	ipcomp_scratches = scratches;
> 
> This patch is broken because on error we will always call
> ipcomp_free_scratches which frees any partially allocated memory
> and restores ipcomp_scratch_users to zero.
> 
> With this patch ipcomp_scratch_users will turn negative on error.
> 
> Cheers,

Thanks for the review. I think it can be fixed by assigning NULL in 
ipcomp_scratches when the allocation fails as ipcomp_free_scratches
checks for it. I shall follow this email with a v2 shortly.

thanks,
   -- Khalid Masum

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2] xfrm: ipcomp: Update ipcomp_scratches with NULL if alloc fails
  2019-02-16  7:01 WARNING in __vunmap syzbot
                   ` (2 preceding siblings ...)
  2022-08-31  1:41 ` [PATCH] xfrm: Don't increase scratch users if allocation fails Khalid Masum
@ 2022-08-31 14:29 ` Khalid Masum
  2022-08-31 14:58   ` Greg KH
  2022-09-01  4:03 ` [PATCH v3] xfrm: Update ipcomp_scratches with NULL if not allocated Khalid Masum
  2022-09-01  7:12 ` [PATCH v4] xfrm: Update ipcomp_scratches with NULL when freed Khalid Masum
  5 siblings, 1 reply; 13+ messages in thread
From: Khalid Masum @ 2022-08-31 14:29 UTC (permalink / raw)
  To: Herbert Xu, netdev, linux-kernel, syzkaller-bugs
  Cc: Steffen Klassert, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, linux-kernel-mentees, Shuah Khan,
	syzbot+5ec9bb042ddfe9644773, Khalid Masum

Currently if ipcomp_alloc_scratches() fails to allocate memory
ipcomp_scratches holds obsolete address. So when we try to free the
percpu scratches using ipcomp_free_scratches() it tries to vfree non
existent vm area. Described below:

static void * __percpu *ipcomp_alloc_scratches(void)
{
	...
	scratches = alloc_percpu(void *);
        if (!scratches)
                return NULL;
ipcomp_scratches does not know about this allocation failure.
Therefore holding the old obsolete address.
        ...
}

So when we free,

static void ipcomp_free_scratches(void)
{
	...

        scratches = ipcomp_scratches;
Receiving obsolete addresses from ipcomp_scratches
        
	if (!scratches)
                return;

        for_each_possible_cpu(i)
               vfree(*per_cpu_ptr(scratches, i));
Trying to free non existent page, causing warning.

        ...
}

Fix this breakage by updating ipcomp_scratches with NULL if
the above mentioned allocation fails.

Reported-and-tested-by: syzbot+5ec9bb042ddfe9644773@syzkaller.appspotmail.com
Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>

---
diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c
index cb40ff0ff28d..17815cde8a7f 100644
--- a/net/xfrm/xfrm_ipcomp.c
+++ b/net/xfrm/xfrm_ipcomp.c
@@ -215,7 +215,7 @@ static void * __percpu *ipcomp_alloc_scratches(void)
 
 	scratches = alloc_percpu(void *);
 	if (!scratches)
-		return NULL;
+		return ipcomp_scratches = NULL;
 
 	ipcomp_scratches = scratches;
 

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] xfrm: ipcomp: Update ipcomp_scratches with NULL if alloc fails
  2022-08-31 14:29 ` [PATCH v2] xfrm: ipcomp: Update ipcomp_scratches with NULL if alloc fails Khalid Masum
@ 2022-08-31 14:58   ` Greg KH
  0 siblings, 0 replies; 13+ messages in thread
From: Greg KH @ 2022-08-31 14:58 UTC (permalink / raw)
  To: Khalid Masum
  Cc: Herbert Xu, netdev, linux-kernel, syzkaller-bugs,
	Steffen Klassert, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	linux-kernel-mentees, David S. Miller,
	syzbot+5ec9bb042ddfe9644773

On Wed, Aug 31, 2022 at 08:29:38PM +0600, Khalid Masum wrote:
> Currently if ipcomp_alloc_scratches() fails to allocate memory
> ipcomp_scratches holds obsolete address. So when we try to free the
> percpu scratches using ipcomp_free_scratches() it tries to vfree non
> existent vm area. Described below:
> 
> static void * __percpu *ipcomp_alloc_scratches(void)
> {
> 	...
> 	scratches = alloc_percpu(void *);
>         if (!scratches)
>                 return NULL;
> ipcomp_scratches does not know about this allocation failure.
> Therefore holding the old obsolete address.
>         ...
> }
> 
> So when we free,
> 
> static void ipcomp_free_scratches(void)
> {
> 	...
> 
>         scratches = ipcomp_scratches;
> Receiving obsolete addresses from ipcomp_scratches
>         
> 	if (!scratches)
>                 return;
> 
>         for_each_possible_cpu(i)
>                vfree(*per_cpu_ptr(scratches, i));
> Trying to free non existent page, causing warning.
> 
>         ...
> }
> 
> Fix this breakage by updating ipcomp_scratches with NULL if
> the above mentioned allocation fails.
> 
> Reported-and-tested-by: syzbot+5ec9bb042ddfe9644773@syzkaller.appspotmail.com
> Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>
> 
> ---
> diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c
> index cb40ff0ff28d..17815cde8a7f 100644
> --- a/net/xfrm/xfrm_ipcomp.c
> +++ b/net/xfrm/xfrm_ipcomp.c
> @@ -215,7 +215,7 @@ static void * __percpu *ipcomp_alloc_scratches(void)
>  
>  	scratches = alloc_percpu(void *);
>  	if (!scratches)
> -		return NULL;
> +		return ipcomp_scratches = NULL;
>  
>  	ipcomp_scratches = scratches;
>  

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/SubmittingPatches for what needs to be done
  here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3] xfrm: Update ipcomp_scratches with NULL if not allocated
  2019-02-16  7:01 WARNING in __vunmap syzbot
                   ` (3 preceding siblings ...)
  2022-08-31 14:29 ` [PATCH v2] xfrm: ipcomp: Update ipcomp_scratches with NULL if alloc fails Khalid Masum
@ 2022-09-01  4:03 ` Khalid Masum
  2022-09-01  4:17   ` Herbert Xu
  2022-09-01  7:12 ` [PATCH v4] xfrm: Update ipcomp_scratches with NULL when freed Khalid Masum
  5 siblings, 1 reply; 13+ messages in thread
From: Khalid Masum @ 2022-09-01  4:03 UTC (permalink / raw)
  To: Herbert Xu, netdev, linux-kernel, syzkaller-bugs
  Cc: Steffen Klassert, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, linux-kernel-mentees, Shuah Khan,
	syzbot+5ec9bb042ddfe9644773, Khalid Masum

Currently if ipcomp_alloc_scratches() fails to allocate memory
ipcomp_scratches holds obsolete address. So when we try to free the
percpu scratches using ipcomp_free_scratches() it tries to vfree non
existent vm area. Described below:

static void * __percpu *ipcomp_alloc_scratches(void)
{
        ...
        scratches = alloc_percpu(void *);
        if (!scratches)
                return NULL;
ipcomp_scratches does not know about this allocation failure.
Therefore holding the old obsolete address.
        ...
}

So when we free,

static void ipcomp_free_scratches(void)
{
        ...
        scratches = ipcomp_scratches;
Assigning obsolete addresses from ipcomp_scratches

        if (!scratches)
                return;

        for_each_possible_cpu(i)
               vfree(*per_cpu_ptr(scratches, i));
Trying to free non existent page, causing warning: trying to vfree
existent vm area.
        ...
}


Fix this breakage by: 
(1) Update ipcomp_scratches with NULL if the above mentioned 
allocation fails.
(2) Update ipcomp_scrtches with NULL when scratches is freed

Reported-by: syzbot+5ec9bb042ddfe9644773@syzkaller.appspotmail.com
Tested-by: syzbot+5ec9bb042ddfe9644773@syzkaller.appspotmail.com
Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>
---
Changes since v2:
- Set ipcomp_scratches to NULL when scratches is freed.
- Update commit message.
- v2 Link: https://lore.kernel.org/lkml/20220831142938.5882-1-khalid.masum.92@gmail.com/

Changes since v1:
- Instead of altering usercount, update ipcomp_scratches to NULL
- Update commit message.
- v1 Link: https://lore.kernel.org/lkml/20220831014126.6708-1-khalid.masum.92@gmail.com/

diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c
index cb40ff0ff28d..3774d07c5819 100644
--- a/net/xfrm/xfrm_ipcomp.c
+++ b/net/xfrm/xfrm_ipcomp.c
@@ -203,6 +203,7 @@ static void ipcomp_free_scratches(void)
 		vfree(*per_cpu_ptr(scratches, i));
 
 	free_percpu(scratches);
+	ipcomp_scratches = NULL;
 }
 
 static void * __percpu *ipcomp_alloc_scratches(void)
@@ -215,7 +216,7 @@ static void * __percpu *ipcomp_alloc_scratches(void)
 
 	scratches = alloc_percpu(void *);
 	if (!scratches)
-		return NULL;
+		return ipcomp_scratches = NULL;
 
 	ipcomp_scratches = scratches;
 

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] xfrm: Update ipcomp_scratches with NULL if not allocated
  2022-09-01  4:03 ` [PATCH v3] xfrm: Update ipcomp_scratches with NULL if not allocated Khalid Masum
@ 2022-09-01  4:17   ` Herbert Xu
  2022-09-01  7:03     ` Khalid Masum
  0 siblings, 1 reply; 13+ messages in thread
From: Herbert Xu @ 2022-09-01  4:17 UTC (permalink / raw)
  To: Khalid Masum
  Cc: netdev, linux-kernel, syzkaller-bugs, Steffen Klassert,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	linux-kernel-mentees, Shuah Khan, syzbot+5ec9bb042ddfe9644773

On Thu, Sep 01, 2022 at 10:03:07AM +0600, Khalid Masum wrote:
> 
> diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c
> index cb40ff0ff28d..3774d07c5819 100644
> --- a/net/xfrm/xfrm_ipcomp.c
> +++ b/net/xfrm/xfrm_ipcomp.c
> @@ -203,6 +203,7 @@ static void ipcomp_free_scratches(void)
>  		vfree(*per_cpu_ptr(scratches, i));
>  
>  	free_percpu(scratches);
> +	ipcomp_scratches = NULL;
>  }

Good catch! This is probably the root cause of all the crashes.

>  static void * __percpu *ipcomp_alloc_scratches(void)
> @@ -215,7 +216,7 @@ static void * __percpu *ipcomp_alloc_scratches(void)
>  
>  	scratches = alloc_percpu(void *);
>  	if (!scratches)
> -		return NULL;
> +		return ipcomp_scratches = NULL;

This is unnecessary as with your first hunk, ipcomp_scratches
is guaranteed to be NULL.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] xfrm: Update ipcomp_scratches with NULL if not allocated
  2022-09-01  4:17   ` Herbert Xu
@ 2022-09-01  7:03     ` Khalid Masum
  0 siblings, 0 replies; 13+ messages in thread
From: Khalid Masum @ 2022-09-01  7:03 UTC (permalink / raw)
  To: Herbert Xu
  Cc: open list:NETWORKING [GENERAL],
	Linux Kernel Mailing List, syzkaller-bugs, Steffen Klassert,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	linux-kernel-mentees, Shuah Khan, syzbot+5ec9bb042ddfe9644773

On Thu, Sep 1, 2022 at 10:18 AM Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Thu, Sep 01, 2022 at 10:03:07AM +0600, Khalid Masum wrote:
> >
> > diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c
> > index cb40ff0ff28d..3774d07c5819 100644
> > --- a/net/xfrm/xfrm_ipcomp.c
> > +++ b/net/xfrm/xfrm_ipcomp.c
> > @@ -203,6 +203,7 @@ static void ipcomp_free_scratches(void)
> >               vfree(*per_cpu_ptr(scratches, i));
> >
> >       free_percpu(scratches);
> > +     ipcomp_scratches = NULL;
> >  }
>
> Good catch! This is probably the root cause of all the crashes.
>
> >  static void * __percpu *ipcomp_alloc_scratches(void)
> > @@ -215,7 +216,7 @@ static void * __percpu *ipcomp_alloc_scratches(void)
> >
> >       scratches = alloc_percpu(void *);
> >       if (!scratches)
> > -             return NULL;
> > +             return ipcomp_scratches = NULL;
>
> This is unnecessary as with your first hunk, ipcomp_scratches
> is guaranteed to be NULL.
>
> Thanks,
> --

You are right. Instead of setting it to NULL at both places, it makes
more sense to
do it when memory is freed.

I shall send a v4 with the suggested change.

thanks,
 -- Khalid Masum

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v4] xfrm: Update ipcomp_scratches with NULL when freed
  2019-02-16  7:01 WARNING in __vunmap syzbot
                   ` (4 preceding siblings ...)
  2022-09-01  4:03 ` [PATCH v3] xfrm: Update ipcomp_scratches with NULL if not allocated Khalid Masum
@ 2022-09-01  7:12 ` Khalid Masum
  2022-09-01  7:48   ` Herbert Xu
  5 siblings, 1 reply; 13+ messages in thread
From: Khalid Masum @ 2022-09-01  7:12 UTC (permalink / raw)
  To: Herbert Xu, netdev, linux-kernel, syzkaller-bugs
  Cc: Steffen Klassert, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, linux-kernel-mentees, Shuah Khan,
	syzbot+5ec9bb042ddfe9644773, Khalid Masum

Currently if ipcomp_alloc_scratches() fails to allocate memory
ipcomp_scratches holds obsolete address. So when we try to free the
percpu scratches using ipcomp_free_scratches() it tries to vfree non
existent vm area. Described below:

static void * __percpu *ipcomp_alloc_scratches(void)
{
        ...
        scratches = alloc_percpu(void *);
        if (!scratches)
                return NULL;
ipcomp_scratches does not know about this allocation failure.
Therefore holding the old obsolete address.
        ...
}

So when we free,

static void ipcomp_free_scratches(void)
{
        ...
        scratches = ipcomp_scratches;
Assigning obsolete address from ipcomp_scratches

        if (!scratches)
                return;

        for_each_possible_cpu(i)
               vfree(*per_cpu_ptr(scratches, i));
Trying to free non existent page, causing warning: trying to vfree
existent vm area.
        ...
}

Fix this breakage by updating ipcomp_scrtches with NULL when scratches
is freed

Suggested-by: Herbert Xu <herbert@gondor.apana.org.au>
Reported-by: syzbot+5ec9bb042ddfe9644773@syzkaller.appspotmail.com
Tested-by: syzbot+5ec9bb042ddfe9644773@syzkaller.appspotmail.com
Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>
---
Changes since v3:
- Update ipcomp_scratches to NULL when freed only
- Link: https://lore.kernel.org/lkml/20220901040307.4674-1-khalid.masum.92@gmail.com/

Changes since v2:
- Set ipcomp_scratches to NULL when scratches is freed.
- Update commit message.
- v2 Link: https://lore.kernel.org/lkml/20220831142938.5882-1-khalid.masum.92@gmail.com/

Changes since v1:
- Instead of altering usercount, update ipcomp_scratches to NULL
- Update commit message.
- v1 Link: https://lore.kernel.org/lkml/20220831014126.6708-1-khalid.masum.92@gmail.com/

diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c
index cb40ff0ff28d..3774d07c5819 100644
--- a/net/xfrm/xfrm_ipcomp.c
+++ b/net/xfrm/xfrm_ipcomp.c
@@ -203,6 +203,7 @@ static void ipcomp_free_scratches(void)
 		vfree(*per_cpu_ptr(scratches, i));
 
 	free_percpu(scratches);
+	ipcomp_scratches = NULL;
 }
 
 static void * __percpu *ipcomp_alloc_scratches(void)

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v4] xfrm: Update ipcomp_scratches with NULL when freed
  2022-09-01  7:12 ` [PATCH v4] xfrm: Update ipcomp_scratches with NULL when freed Khalid Masum
@ 2022-09-01  7:48   ` Herbert Xu
  0 siblings, 0 replies; 13+ messages in thread
From: Herbert Xu @ 2022-09-01  7:48 UTC (permalink / raw)
  To: Khalid Masum
  Cc: netdev, linux-kernel, syzkaller-bugs, Steffen Klassert,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	linux-kernel-mentees, Shuah Khan, syzbot+5ec9bb042ddfe9644773

On Thu, Sep 01, 2022 at 01:12:10PM +0600, Khalid Masum wrote:
> Currently if ipcomp_alloc_scratches() fails to allocate memory
> ipcomp_scratches holds obsolete address. So when we try to free the
> percpu scratches using ipcomp_free_scratches() it tries to vfree non
> existent vm area. Described below:
> 
> static void * __percpu *ipcomp_alloc_scratches(void)
> {
>         ...
>         scratches = alloc_percpu(void *);
>         if (!scratches)
>                 return NULL;
> ipcomp_scratches does not know about this allocation failure.
> Therefore holding the old obsolete address.
>         ...
> }
> 
> So when we free,
> 
> static void ipcomp_free_scratches(void)
> {
>         ...
>         scratches = ipcomp_scratches;
> Assigning obsolete address from ipcomp_scratches
> 
>         if (!scratches)
>                 return;
> 
>         for_each_possible_cpu(i)
>                vfree(*per_cpu_ptr(scratches, i));
> Trying to free non existent page, causing warning: trying to vfree
> existent vm area.
>         ...
> }
> 
> Fix this breakage by updating ipcomp_scrtches with NULL when scratches
> is freed
> 
> Suggested-by: Herbert Xu <herbert@gondor.apana.org.au>
> Reported-by: syzbot+5ec9bb042ddfe9644773@syzkaller.appspotmail.com
> Tested-by: syzbot+5ec9bb042ddfe9644773@syzkaller.appspotmail.com
> Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>

Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-09-01  7:50 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-16  7:01 WARNING in __vunmap syzbot
2019-09-08  7:05 ` syzbot
2021-08-04  5:35 ` [syzbot] " syzbot
2022-08-31  1:41 ` [PATCH] xfrm: Don't increase scratch users if allocation fails Khalid Masum
2022-08-31  9:13   ` Herbert Xu
2022-08-31 12:01     ` Khalid Masum
2022-08-31 14:29 ` [PATCH v2] xfrm: ipcomp: Update ipcomp_scratches with NULL if alloc fails Khalid Masum
2022-08-31 14:58   ` Greg KH
2022-09-01  4:03 ` [PATCH v3] xfrm: Update ipcomp_scratches with NULL if not allocated Khalid Masum
2022-09-01  4:17   ` Herbert Xu
2022-09-01  7:03     ` Khalid Masum
2022-09-01  7:12 ` [PATCH v4] xfrm: Update ipcomp_scratches with NULL when freed Khalid Masum
2022-09-01  7:48   ` Herbert Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).