linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michael Kelley <mikelley@microsoft.com>
To: vkuznets <vkuznets@redhat.com>,
	"linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Wei Liu <wei.liu@kernel.org>,
	Tianyu Lan <Tianyu.Lan@microsoft.com>,
	KY Srinivasan <kys@microsoft.com>,
	Haiyang Zhang <haiyangz@microsoft.com>,
	Stephen Hemminger <sthemmin@microsoft.com>,
	"Andrea Parri (Microsoft)" <parri.andrea@gmail.com>
Subject: RE: [PATCH 5/5] Drivers: hv: check VMBus messages lengths
Date: Fri, 3 Apr 2020 02:00:02 +0000	[thread overview]
Message-ID: <MW2PR2101MB1052E7B2458F489E0123C1D9D7C70@MW2PR2101MB1052.namprd21.prod.outlook.com> (raw)
In-Reply-To: <20200401103816.1406642-2-vkuznets@redhat.com>

From: Vitaly Kuznetsov <vkuznets@redhat.com>  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.

> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  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@microsoft.com>

  reply	other threads:[~2020-04-03  2:00 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-01 10:36 [PATCH 0/5] Drivers: hv: cleanup VMBus messages handling Vitaly Kuznetsov
2020-04-01 10:36 ` [PATCH 1/5] Drivers: hv: copy from message page only what's needed Vitaly Kuznetsov
2020-04-02 14:46   ` [EXTERNAL] " 163
2020-04-02 16:26     ` Vitaly Kuznetsov
2020-04-03  1:52       ` Michael Kelley
2020-04-03  7:03         ` Vitaly Kuznetsov
2020-04-01 10:36 ` [PATCH 2/5] Drivers: hv: allocate the exact needed memory for messages Vitaly Kuznetsov
2020-04-03  1:54   ` Michael Kelley
2020-04-01 10:36 ` [PATCH 3/5] Drivers: hv: avoid passing opaque pointer to vmbus_onmessage() Vitaly Kuznetsov
2020-04-03  1:55   ` Michael Kelley
2020-04-01 10:38 ` [PATCH 4/5] Drivers: hv: make sure that 'struct vmbus_channel_message_header' compiles correctly Vitaly Kuznetsov
2020-04-01 10:38   ` [PATCH 5/5] Drivers: hv: check VMBus messages lengths Vitaly Kuznetsov
2020-04-03  2:00     ` Michael Kelley [this message]
2020-04-03  7:07       ` Vitaly Kuznetsov
2020-04-03  1:56   ` [PATCH 4/5] Drivers: hv: make sure that 'struct vmbus_channel_message_header' compiles correctly Michael Kelley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=MW2PR2101MB1052E7B2458F489E0123C1D9D7C70@MW2PR2101MB1052.namprd21.prod.outlook.com \
    --to=mikelley@microsoft.com \
    --cc=Tianyu.Lan@microsoft.com \
    --cc=haiyangz@microsoft.com \
    --cc=kys@microsoft.com \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=parri.andrea@gmail.com \
    --cc=sthemmin@microsoft.com \
    --cc=vkuznets@redhat.com \
    --cc=wei.liu@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).