releases/6.8.6/vmci-fix-memcpy-run-time-warning-in-dg_dispatch_as_h.patch

   1 From 7279cf06725cf61c6e27be5556c315258b08dcbf Mon Sep 17 00:00:00 2001
   2 From: Sasha Levin <sashal@kernel.org>
   3 Date: Fri, 5 Jan 2024 08:40:00 -0800
   4 Subject: VMCI: Fix memcpy() run-time warning in dg_dispatch_as_host()
   5
   6 From: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
   7
   8 [ Upstream commit 19b070fefd0d024af3daa7329cbc0d00de5302ec ]
   9
  10 Syzkaller hit 'WARNING in dg_dispatch_as_host' bug.
  11
  12 memcpy: detected field-spanning write (size 56) of single field "&dg_info->msg"
  13 at drivers/misc/vmw_vmci/vmci_datagram.c:237 (size 24)
  14
  15 WARNING: CPU: 0 PID: 1555 at drivers/misc/vmw_vmci/vmci_datagram.c:237
  16 dg_dispatch_as_host+0x88e/0xa60 drivers/misc/vmw_vmci/vmci_datagram.c:237
  17
  18 Some code commentry, based on my understanding:
  19
  20 544 #define VMCI_DG_SIZE(_dg) (VMCI_DG_HEADERSIZE + (size_t)(_dg)->payload_size)
  21 /// This is 24 + payload_size
  22
  23 memcpy(&dg_info->msg, dg, dg_size);
  24         Destination = dg_info->msg ---> this is a 24 byte
  25                                         structure(struct vmci_datagram)
  26         Source = dg --> this is a 24 byte structure (struct vmci_datagram)
  27         Size = dg_size = 24 + payload_size
  28
  29 {payload_size = 56-24 =32} -- Syzkaller managed to set payload_size to 32.
  30
  31  35 struct delayed_datagram_info {
  32  36         struct datagram_entry *entry;
  33  37         struct work_struct work;
  34  38         bool in_dg_host_queue;
  35  39         /* msg and msg_payload must be together. */
  36  40         struct vmci_datagram msg;
  37  41         u8 msg_payload[];
  38  42 };
  39
  40 So those extra bytes of payload are copied into msg_payload[], a run time
  41 warning is seen while fuzzing with Syzkaller.
  42
  43 One possible way to fix the warning is to split the memcpy() into
  44 two parts -- one -- direct assignment of msg and second taking care of payload.
  45
  46 Gustavo quoted:
  47 "Under FORTIFY_SOURCE we should not copy data across multiple members
  48 in a structure."
  49
  50 Reported-by: syzkaller <syzkaller@googlegroups.com>
  51 Suggested-by: Vegard Nossum <vegard.nossum@oracle.com>
  52 Suggested-by: Gustavo A. R. Silva <gustavoars@kernel.org>
  53 Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
  54 Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
  55 Reviewed-by: Kees Cook <keescook@chromium.org>
  56 Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
  57 Link: https://lore.kernel.org/r/20240105164001.2129796-2-harshit.m.mogalapalli@oracle.com
  58 Signed-off-by: Kees Cook <keescook@chromium.org>
  59 Signed-off-by: Sasha Levin <sashal@kernel.org>
  60 ---
  61  drivers/misc/vmw_vmci/vmci_datagram.c | 3 ++-
  62  1 file changed, 2 insertions(+), 1 deletion(-)
  63
  64 diff --git a/drivers/misc/vmw_vmci/vmci_datagram.c b/drivers/misc/vmw_vmci/vmci_datagram.c
  65 index f50d22882476f..d1d8224c8800c 100644
  66 --- a/drivers/misc/vmw_vmci/vmci_datagram.c
  67 +++ b/drivers/misc/vmw_vmci/vmci_datagram.c
  68 @@ -234,7 +234,8 @@ static int dg_dispatch_as_host(u32 context_id, struct vmci_datagram *dg)
  69
  70                         dg_info->in_dg_host_queue = true;
  71                         dg_info->entry = dst_entry;
  72 -                       memcpy(&dg_info->msg, dg, dg_size);
  73 +                       dg_info->msg = *dg;
  74 +                       memcpy(&dg_info->msg_payload, dg + 1, dg->payload_size);
  75
  76                         INIT_WORK(&dg_info->work, dg_delayed_dispatch);
  77                         schedule_work(&dg_info->work);
  78 --
  79 2.43.0
  80