Messages in this thread |  | | Subject | Re: [EXTERNAL] [PATCH 1/5] Drivers: hv: copy from message page only what's needed | From | 163 <> | Date | Thu, 2 Apr 2020 22:46:53 +0800 |
| |
On 4/1/2020 6:36 PM, Vitaly Kuznetsov wrote: > Hyper-V Interrupt Message Page (SIMP) has 16 256-byte slots for > messages. Each message comes with a header (16 bytes) which specifies the > payload length (up to 240 bytes). vmbus_on_msg_dpc(), however, doesn't > look at the real message length and copies the whole slot to a temporary > buffer before passing it to message handlers. This is potentially dangerous > as hypervisor doesn't have to clean the whole slot when putting a new > message there and a message handler can get access to some data which > belongs to a previous message. > > Note, this is not currently a problem because all message handlers are > in-kernel but eventually we may e.g. get this exported to userspace. > > Note also, that this is not a performance critical path: messages (unlike > events) represent rare events so it doesn't really matter (from performance > point of view) if we copy too much. > > Fix the issue by taking into account the real message length. The temporary > buffer allocated by vmbus_on_msg_dpc() remains fixed size for now. > > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > drivers/hv/vmbus_drv.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c > index 029378c27421..2b5572146358 100644 > --- a/drivers/hv/vmbus_drv.c > +++ b/drivers/hv/vmbus_drv.c > @@ -1043,7 +1043,8 @@ void vmbus_on_msg_dpc(unsigned long data) > return; > > INIT_WORK(&ctx->work, vmbus_onmessage_work); > - memcpy(&ctx->msg, msg, sizeof(*msg)); > + memcpy(&ctx->msg, msg, sizeof(msg->header) + > + msg->header.payload_size); >
Hi Vitaly: I think we still need to check whether the payload_size passed from Hyper-V is valid or not here to avoid cross-border issue before doing copying.
|  |