[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c9aff51d-918a-49d7-ae31-395cc6a5881b@oracle.com>
Date: Mon, 8 Jan 2024 23:01:36 +0530
From: Harshit Mogalapalli <harshit.m.mogalapalli@...cle.com>
To: "Gustavo A. R. Silva" <gustavo@...eddedor.com>,
Dan Carpenter <dan.carpenter@...aro.org>
Cc: linux-hardening@...r.kernel.org, keescook@...omium.org, error27@...il.com,
gustavoars@...nel.org, Bryan Tan <bryantan@...are.com>,
Vishnu Dasa <vdasa@...are.com>,
VMware PV-Drivers Reviewers <pv-drivers@...are.com>,
Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-kernel@...r.kernel.org, vegard.nossum@...cle.com,
darren.kenny@...cle.com, syzkaller <syzkaller@...glegroups.com>
Subject: Re: [PATCH v2 2/2] VMCI: Fix memcpy() run-time warning in
dg_dispatch_as_host()
Hi Gustavo,
On 08/01/24 10:33 pm, Gustavo A. R. Silva wrote:
>
>
> On 1/8/24 01:33, Dan Carpenter wrote:
>> On Fri, Jan 05, 2024 at 08:40:00AM -0800, Harshit Mogalapalli wrote:
>>> Syzkaller hit 'WARNING in dg_dispatch_as_host' bug.
>>>
>>> memcpy: detected field-spanning write (size 56) of single field
>>> "&dg_info->msg"
>>> at drivers/misc/vmw_vmci/vmci_datagram.c:237 (size 24)
>>>
>>> WARNING: CPU: 0 PID: 1555 at drivers/misc/vmw_vmci/vmci_datagram.c:237
>>> dg_dispatch_as_host+0x88e/0xa60
>>> drivers/misc/vmw_vmci/vmci_datagram.c:237
>>>
>>> Some code commentry, based on my understanding:
>>>
>>> 544 #define VMCI_DG_SIZE(_dg) (VMCI_DG_HEADERSIZE +
>>> (size_t)(_dg)->payload_size)
>>> /// This is 24 + payload_size
>>>
>>> memcpy(&dg_info->msg, dg, dg_size);
>>> Destination = dg_info->msg ---> this is a 24 byte
>>> structure(struct vmci_datagram)
>>> Source = dg --> this is a 24 byte structure (struct vmci_datagram)
>>> Size = dg_size = 24 + payload_size
>>>
>>> {payload_size = 56-24 =32} -- Syzkaller managed to set payload_size
>>> to 32.
>>>
>>> 35 struct delayed_datagram_info {
>>> 36 struct datagram_entry *entry;
>>> 37 struct work_struct work;
>>> 38 bool in_dg_host_queue;
>>> 39 /* msg and msg_payload must be together. */
>>> 40 struct vmci_datagram msg;
>>> 41 u8 msg_payload[];
>>> 42 };
>>>
>>> So those extra bytes of payload are copied into msg_payload[], a run
>>> time
>>> warning is seen while fuzzing with Syzkaller.
>>>
>>> One possible way to fix the warning is to split the memcpy() into
>>> two parts -- one -- direct assignment of msg and second taking care
>>> of payload.
>>>
>>> Gustavo quoted:
>>> "Under FORTIFY_SOURCE we should not copy data across multiple members
>>> in a structure."
>>>
>>> Reported-by: syzkaller <syzkaller@...glegroups.com>
>>> Suggested-by: Vegard Nossum <vegard.nossum@...cle.com>
>>> Suggested-by: Gustavo A. R. Silva <gustavoars@...nel.org>
>>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@...cle.com>
>>> ---
>>> This patch is only tested with the C reproducer, not any testing
>>> specific to driver is done.
>>>
>>> v1->v2: ( Suggestions from Gustavo )
>>> 1. Change the commit message false positive --> legitimate
>>> warning.
>>
>> The commit message is fine.
>>
>> Reviewed-by: Dan Carpenter <dan.carpenter@...aro.org>
>>
>> But, I mean, it's not really "legitimate". It meets the fortify source
>> heuristic, but it's still a false positive. Fortify source is
>> *supposed* to find memory corruption bugs and this is not a memory
>> corruption bug. It's just that these days we have to treat foritify
>> false positives as crashing bugs because people enable it and we have to
>> fix it.
>>
>> Let's not pretend that fortify has helped us in this situation, it has
>> caused us a problem. It has taken valid code and created a crashing
>> bug. I'm not saying that the cost isn't worth it, but let's not pretend.
>>
>
> It's a "legitimate warning" (which differs from a "legitimate memory
> corruption bug") in the sense that the feature is doing what it's
> supposed to do: reporting a write beyond the boundaries of a field/member
> in a structure.
>
> Is that simple. I don't see the "pretense" here.
>
> BTW, is this _warning_ really causing a crash?
>
I think we can say this can cause a crash in a way, WARN_ONCE() is still
a WARNING and we can have systems with panic_on_warn set.
Eg: on how a crash would look like when panic_on_warn is set.
[ 173.803078] ------------[ cut here ]------------
[ 173.806961] memcpy: detected field-spanning write (size 56) of single
field "&dg_info->msg" at drivers/misc/vmw_vmci/vmci_datagram.c:237 (size 24)
[ 173.817612] WARNING: CPU: 4 PID: 9003 at
drivers/misc/vmw_vmci/vmci_datagram.c:237 dg_dispatch_as_host+0x88e/0xa60
[ 173.826031] Modules linked in:
[ 173.828765] CPU: 4 PID: 9003 Comm: r Not tainted 6.7.0-rc7 #6
[ 173.833502] Hardware name: Red Hat KVM, BIOS 1.16.1-1.el9 04/01/2014
[ 173.838689] RIP: 0010:dg_dispatch_as_host+0x88e/0xa60
[ 173.842953] Code: fe ff ff e8 a4 61 70 fa b9 18 00 00 00 48 89 de 48
c7 c2 e0 c8 20 92 48 c7 c7 60 c9 20 92 c6 05 e3 62 22 12 01 e8 92 c2 38
fa <0f> 0b e9 82 fe ff ff e8 76 61 70 fa e8 71 61 70 fa 48 8d 7d 0c 48
[ 173.857632] RSP: 0018:ffff88810362fb10 EFLAGS: 00010246
[ 173.862078] RAX: 0000000000000000 RBX: 0000000000000038 RCX:
0000000000000000
[ 173.867885] RDX: 0000000000000000 RSI: 0000000000000000 RDI:
0000000000000000
[ 173.873689] RBP: ffff888118f94680 R08: 0000000000000000 R09:
0000000000000000
[ 173.879503] R10: 0000000000000000 R11: 0000000000000000 R12:
ffff888118f71130
[ 173.885317] R13: ffff888118f71100 R14: 0000000000000000 R15:
0000000000000000
[ 173.891124] FS: 00007fced2868740(0000) GS:ffff8881f4200000(0000)
knlGS:0000000000000000
[ 173.897658] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 173.902397] CR2: 0000000020000000 CR3: 00000001198d2000 CR4:
00000000000006f0
[ 173.908222] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[ 173.914073] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
0000000000000400
[ 173.919901] Call Trace:
[ 173.922117] <TASK>
[ 173.924075] ? show_regs+0x9b/0xb0
[ 173.927151] ? __warn+0xeb/0x2c0
[ 173.929981] ? dg_dispatch_as_host+0x88e/0xa60
[ 173.933769] ? report_bug+0x313/0x410
[ 173.937010] ? dg_dispatch_as_host+0x88e/0xa60
[ 173.940804] ? handle_bug+0x9d/0x130
[ 173.943922] ? exc_invalid_op+0x36/0x80
[ 173.947195] ? asm_exc_invalid_op+0x1a/0x20
[ 173.950768] ? dg_dispatch_as_host+0x88e/0xa60
[ 173.954524] vmci_datagram_dispatch+0x1da/0x230
[ 173.958368] ? __pfx_vmci_datagram_dispatch+0x10/0x10
[ 173.962583] vmci_datagram_send+0x2d/0x50
[ 173.966006] vmci_transport_dgram_enqueue+0x2e2/0x410
[ 173.970228] ? __pfx_vmci_transport_dgram_allow+0x10/0x10
[ 173.974689] vsock_dgram_sendmsg+0x391/0x610
[ 173.978336] ? __pfx_vsock_dgram_sendmsg+0x10/0x10
[ 173.982568] __sys_sendto+0x4dc/0x540
[ 173.985767] ? __pfx___sys_sendto+0x10/0x10
[ 173.989289] ? __pfx_vsock_dgram_connect+0x10/0x10
[ 173.993312] ? selinux_netlbl_socket_connect+0x37/0x50
[ 173.997588] ? selinux_socket_connect+0x76/0xa0
[ 174.001449] ? __sys_connect_file+0x5d/0x1b0
[ 174.005078] ? __sys_connect+0x113/0x1b0
[ 174.008420] ? __pfx___sys_connect+0x10/0x10
[ 174.012031] ? count_memcg_events.constprop.0+0x48/0x60
[ 174.016346] ? handle_mm_fault+0x2c8/0x910
[ 174.019797] ? ktime_get_coarse_real_ts64+0xa0/0xf0
[ 174.023862] ? __audit_syscall_entry+0x393/0x4f0
[ 174.027769] __x64_sys_sendto+0xe9/0x1d0
[ 174.031090] ? syscall_trace_enter.constprop.0+0x138/0x1b0
[ 174.035604] do_syscall_64+0x45/0x100
[ 174.038765] entry_SYSCALL_64_after_hwframe+0x6e/0x76
[ 174.042953] RIP: 0033:0x7fced220eca3
[ 174.046040] Code: 48 8b 0d 08 83 20 00 f7 d8 64 89 01 48 83 c8 ff c3
66 0f 1f 44 00 00 83 3d 49 c7 20 00 00 75 13 49 89 ca b8 2c 00 00 00 0f
05 <48> 3d 01 f0 ff ff 73 34 c3 48 83 ec 08 e8 2b f7 ff ff 48 89 04 24
[ 174.060689] RSP: 002b:00007ffe5f77d2e8 EFLAGS: 00000246 ORIG_RAX:
000000000000002c
[ 174.066898] RAX: ffffffffffffffda RBX: 0000000000000000 RCX:
00007fced220eca3
[ 174.072766] RDX: 0000000000000020 RSI: 00007ffe5f77d400 RDI:
0000000000000004
[ 174.078573] RBP: 00007ffe5f77d330 R08: 00007ffe5f77d310 R09:
000000000000000c
[ 174.084376] R10: 0000000000000000 R11: 0000000000000246 R12:
0000000000400730
[ 174.090185] R13: 00007ffe5f77e520 R14: 0000000000000000 R15:
0000000000000000
[ 174.096014] </TASK>
[ 174.098021] Kernel panic - not syncing: kernel: panic_on_warn set ...
[ 174.101976] CPU: 4 PID: 9003 Comm: r Not tainted 6.7.0-rc7 #6
[ 174.107921] Hardware name: Red Hat KVM, BIOS 1.16.1-1.el9 04/01/2014
[ 174.112923] Call Trace:
[ 174.113973] <TASK>
[ 174.115922] dump_stack_lvl+0x83/0xb0
[ 174.117975] panic+0x697/0x720
[ 174.121972] ? show_trace_log_lvl+0x3bb/0x520
[ 174.125942] ? __pfx_panic+0x10/0x10
[ 174.128925] ? dg_dispatch_as_host+0x88e/0xa60
[ 174.131924] check_panic_on_warn+0xb6/0xc0
[ 174.133970] __warn+0xf7/0x2c0
[ 174.137970] ? dg_dispatch_as_host+0x88e/0xa60
[ 174.141970] report_bug+0x313/0x410
[ 174.144924] ? dg_dispatch_as_host+0x88e/0xa60
[ 174.148898] handle_bug+0x9d/0x130
[ 174.149968] exc_invalid_op+0x36/0x80
[ 174.153969] asm_exc_invalid_op+0x1a/0x20
[ 174.157970] RIP: 0010:dg_dispatch_as_host+0x88e/0xa60
[ 174.161970] Code: fe ff ff e8 a4 61 70 fa b9 18 00 00 00 48 89 de 48
c7 c2 e0 c8 20 92 48 c7 c7 60 c9 20 92 c6 05 e3 62 22 12 01 e8 92 c2 38
fa <0f> 0b e9 82 fe ff ff e8 76 61 70 fa e8 71 61 70 fa 48 8d 7d 0c 48
[ 174.176923] RSP: 0018:ffff88810362fb10 EFLAGS: 00010246
[ 174.179922] RAX: 0000000000000000 RBX: 0000000000000038 RCX:
0000000000000000
[ 174.185967] RDX: 0000000000000000 RSI: 0000000000000000 RDI:
0000000000000000
[ 174.189966] RBP: ffff888118f94680 R08: 0000000000000000 R09:
0000000000000000
[ 174.197968] R10: 0000000000000000 R11: 0000000000000000 R12:
ffff888118f71130
[ 174.201969] R13: ffff888118f71100 R14: 0000000000000000 R15:
0000000000000000
[ 174.208924] vmci_datagram_dispatch+0x1da/0x230
[ 174.212884] ? __pfx_vmci_datagram_dispatch+0x10/0x10
[ 174.216923] vmci_datagram_send+0x2d/0x50
[ 174.219924] vmci_transport_dgram_enqueue+0x2e2/0x410
[ 174.221966] ? __pfx_vmci_transport_dgram_allow+0x10/0x10
[ 174.227939] vsock_dgram_sendmsg+0x391/0x610
[ 174.230883] ? __pfx_vsock_dgram_sendmsg+0x10/0x10
[ 174.236021] __sys_sendto+0x4dc/0x540
[ 174.237970] ? __pfx___sys_sendto+0x10/0x10
[ 174.241968] ? __pfx_vsock_dgram_connect+0x10/0x10
[ 174.245969] ? selinux_netlbl_socket_connect+0x37/0x50
[ 174.249969] ? selinux_socket_connect+0x76/0xa0
[ 174.253968] ? __sys_connect_file+0x5d/0x1b0
[ 174.257966] ? __sys_connect+0x113/0x1b0
[ 174.259923] ? __pfx___sys_connect+0x10/0x10
[ 174.264922] ? count_memcg_events.constprop.0+0x48/0x60
[ 174.267947] ? handle_mm_fault+0x2c8/0x910
[ 174.269966] ? ktime_get_coarse_real_ts64+0xa0/0xf0
[ 174.275924] ? __audit_syscall_entry+0x393/0x4f0
[ 174.277970] __x64_sys_sendto+0xe9/0x1d0
[ 174.281972] ? syscall_trace_enter.constprop.0+0x138/0x1b0
[ 174.285970] do_syscall_64+0x45/0x100
[ 174.289966] entry_SYSCALL_64_after_hwframe+0x6e/0x76
[ 174.293967] RIP: 0033:0x7fced220eca3
[ 174.297970] Code: 48 8b 0d 08 83 20 00 f7 d8 64 89 01 48 83 c8 ff c3
66 0f 1f 44 00 00 83 3d 49 c7 20 00 00 75 13 49 89 ca b8 2c 00 00 00 0f
05 <48> 3d 01 f0 ff ff 73 34 c3 48 83 ec 08 e8 2b f7 ff ff 48 89 04 24
[ 174.311883] RSP: 002b:00007ffe5f77d2e8 EFLAGS: 00000246 ORIG_RAX:
000000000000002c
[ 174.317968] RAX: ffffffffffffffda RBX: 0000000000000000 RCX:
00007fced220eca3
[ 174.323930] RDX: 0000000000000020 RSI: 00007ffe5f77d400 RDI:
0000000000000004
[ 174.329968] RBP: 00007ffe5f77d330 R08: 00007ffe5f77d310 R09:
000000000000000c
[ 174.333967] R10: 0000000000000000 R11: 0000000000000246 R12:
0000000000400730
[ 174.340923] R13: 00007ffe5f77e520 R14: 0000000000000000 R15:
0000000000000000
[ 174.345969] </TASK>
[ 174.348922] Dumping ftrace buffer:
[ 174.348922] (ftrace buffer empty)
[ 174.348922] Kernel Offset: disabled
[ 174.348922] Rebooting in 86400 seconds..
Thanks,
Harshit
> Thanks
> --
> Gustavo
>
Powered by blists - more mailing lists