diff options
| author | Harshit Mogalapalli <[email protected]> | 2024-01-05 16:40:00 +0000 |
|---|---|---|
| committer | Kees Cook <[email protected]> | 2024-02-01 18:06:42 +0000 |
| commit | 19b070fefd0d024af3daa7329cbc0d00de5302ec (patch) | |
| tree | d6f96a0379e628a5b9160508d4927ad3aec46e3b /lib/string_helpers.c | |
| parent | VMCI: Use struct_size() in kmalloc() (diff) | |
| download | kernel-19b070fefd0d024af3daa7329cbc0d00de5302ec.tar.gz kernel-19b070fefd0d024af3daa7329cbc0d00de5302ec.zip | |
VMCI: Fix memcpy() run-time warning in dg_dispatch_as_host()
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 <[email protected]>
Suggested-by: Vegard Nossum <[email protected]>
Suggested-by: Gustavo A. R. Silva <[email protected]>
Signed-off-by: Harshit Mogalapalli <[email protected]>
Reviewed-by: Gustavo A. R. Silva <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Reviewed-by: Dan Carpenter <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Kees Cook <[email protected]>
Diffstat (limited to 'lib/string_helpers.c')
0 files changed, 0 insertions, 0 deletions
