Re: [PATCH v2 2/2] VMCI: Fix memcpy() run-time warning in dg_dispatch_as_host()

From: Harshit Mogalapalli
Date: Mon Jan 08 2024 - 12:34:33 EST


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@xxxxxxxxxxxxxxxx>
Suggested-by: Vegard Nossum <vegard.nossum@xxxxxxxxxx>
Suggested-by: Gustavo A. R. Silva <gustavoars@xxxxxxxxxx>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@xxxxxxxxxx>
---
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@xxxxxxxxxx>

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