netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stefano Garzarella <sgarzare@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: virtualization@lists.linux-foundation.org,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Jason Wang <jasowang@redhat.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org
Subject: Re: [PATCH] vhost/vsock: accept only packets with the right dst_cid
Date: Thu, 12 Dec 2019 13:36:24 +0100	[thread overview]
Message-ID: <20191212123624.ahyhrny7u6ntn3xt@steredhat> (raw)
In-Reply-To: <20191211110235-mutt-send-email-mst@kernel.org>

On Wed, Dec 11, 2019 at 11:03:07AM -0500, Michael S. Tsirkin wrote:
> On Fri, Dec 06, 2019 at 03:39:12PM +0100, Stefano Garzarella wrote:
> > When we receive a new packet from the guest, we check if the
> > src_cid is correct, but we forgot to check the dst_cid.
> > 
> > The host should accept only packets where dst_cid is
> > equal to the host CID.
> > 
> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> 
> Stefano can you clarify the impact pls?

Sure, I'm sorry I didn't do it earlier.

> E.g. is this needed on stable? Etc.

This is a better analysis (I hope) when there is a malformed guest
that sends a packet with a wrong dst_cid:
- before v5.4 we supported only one transport at runtime, so the sockets
  in the host can only receive packets from guests. In this case, if
  the dst_cid is wrong, maybe the only issue is that the getsockname()
  returns an inconsistent address (the cid returned is the one received
  from the guest)

- from v5.4 we support multi-transport, so the L1 VM (e.g. L0 assigned
  cid 5 to this VM) can have both Guest2Host and Host2Guest transports.
  In this case, we have these possible issues:
  - L2 (or L1) guest can use cid 0, 1, and 2 to reach L1 (or L0),
    instead we should allow only CID_HOST (2) to reach the level below.
    Note: this happens also with not malformed guest that runs Linux v5.4

  - if a malformed L2 guest sends a packet with the wrong dst_cid, for example
    instead of CID_HOST, it uses the cid assigned by L0 to L1 (5 in this
    example), this packets can wrongly queued to a socket on L1 bound to cid 5,
    that only expects connections from L0.

Maybe we really need this only on stable v5.4, but the patch is very simple
and should apply cleanly to all stable branches.

What do you think?

Thanks,
Stefano


  reply	other threads:[~2019-12-12 12:36 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-06 14:39 [PATCH] vhost/vsock: accept only packets with the right dst_cid Stefano Garzarella
2019-12-07 20:00 ` David Miller
2019-12-10 14:05 ` Michael S. Tsirkin
2019-12-10 14:32   ` Stefano Garzarella
2019-12-11 16:03 ` Michael S. Tsirkin
2019-12-12 12:36   ` Stefano Garzarella [this message]
2019-12-12 12:56     ` Michael S. Tsirkin
2019-12-12 13:14       ` Stefano Garzarella
2019-12-12 17:18         ` Stefano Garzarella
2019-12-12 19:01         ` David Miller

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=20191212123624.ahyhrny7u6ntn3xt@steredhat \
    --to=sgarzare@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=stefanha@redhat.com \
    --cc=virtualization@lists.linux-foundation.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).