Michael Kelley <mikelley@xxxxxxxxxxxxx> writes: > From: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> Sent: Wednesday, April 1, 2020 3:38 AM >> >> VMBus message handlers (channel_message_table) receive a pointer to >> 'struct vmbus_channel_message_header' and cast it to a structure of their >> choice, which is sometimes longer than the header. We, however, don't check >> that the message is long enough so in case hypervisor screws up we'll be >> accessing memory beyond what was allocated for temporary buffer. >> >> Previously, we used to always allocate and copy 256 bytes from message page >> to temporary buffer but this is hardly better: in case the message is >> shorter than we expect we'll be trying to consume garbage as some real >> data and no memory guarding technique will be able to identify an issue. >> >> Introduce 'min_payload_len' to 'struct vmbus_channel_message_table_entry' >> and check against it in vmbus_on_msg_dpc(). Note, we can't require the >> exact length as new hypervisor versions may add extra fields to messages, >> we only check that the message is not shorter than we expect. > > This assumes that the current structure definitions don't already > include extra fields that were added in newer versions of Hyper-V. If they did, > the minimum length test could fail on older versions of Hyper-V. But I > looked through the structure definitions and don't see any indication that > such extra fields have been added, so this should be OK. > Yes, my understanding as well. When we decide to extend some of these structures for newer VMbus version we'll have a choice: keep the require length minimal or implement a more somplex check (e.g. add a 'length check' function pointer to vmbus_channel_message_table_entry() -- or pass 'length' to all message handlers and handle it ther). But as we currently have no such cases this will definitely look over-engineered so I decided to go the easiest way. >> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> >> --- >> drivers/hv/channel_mgmt.c | 54 ++++++++++++++++++++++----------------- >> drivers/hv/hyperv_vmbus.h | 1 + >> drivers/hv/vmbus_drv.c | 6 +++++ >> 3 files changed, 37 insertions(+), 24 deletions(-) >> > > Reviewed-by: Michael Kelley <mikelley@xxxxxxxxxxxxx> > Thanks! -- Vitaly
