netdev - RE: [PATCH net-next 3/3] hv_sock: implements Hyper-V transport for Virtual Sockets (AF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <PS1P15301MB0011D3F00204245172E8150EBF840@PS1P15301MB0011.APCP153.PROD.OUTLOOK.COM>
Date:   Tue, 22 Aug 2017 21:40:01 +0000
From:   Dexuan Cui <decui@...rosoft.com>
To:     Stefan Hajnoczi <stefanha@...hat.com>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
        KY Srinivasan <kys@...rosoft.com>,
        "Haiyang Zhang" <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        George Zhang <georgezhang@...are.com>,
        Jorgen Hansen <jhansen@...are.com>,
        Michal Kubecek <mkubecek@...e.cz>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Cathy Avery <cavery@...hat.com>,
        "jasowang@...hat.com" <jasowang@...hat.com>,
        Rolf Neugebauer <rolf.neugebauer@...ker.com>,
        Dave Scott <dave.scott@...ker.com>,
        Marcelo Cerri <marcelo.cerri@...onical.com>,
        "apw@...onical.com" <apw@...onical.com>,
        "olaf@...fle.de" <olaf@...fle.de>,
        "joe@...ches.com" <joe@...ches.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Dan Carpenter <dan.carpenter@...cle.com>
Subject: RE: [PATCH net-next 3/3] hv_sock: implements Hyper-V transport for
 Virtual Sockets (AF_VSOCK)

> From: Stefan Hajnoczi [mailto:stefanha@...hat.com]
> On Fri, Aug 18, 2017 at 10:23:54PM +0000, Dexuan Cui wrote:
> > > > +static bool hvs_stream_allow(u32 cid, u32 port)
> > > > +{
> > > > +	static const u32 valid_cids[] = {
> > > > +		VMADDR_CID_ANY,
> > >
> > > Is this for loopback?
> >
> > No, we don't support lookback in Linux VM, at least for now.
> > In our Linux implementation, Linux VM can only connect to the host, and
> > here when Linux VM calls connect(), I treat  VMADDR_CID_ANY
> > the same as VMADDR_CID_HOST.
> 
> VMCI and virtio-vsock do not treat connect(VMADDR_CID_ANY) the same as
> connect(VMADDR_CID_HOST).  It is an error to connect to VMADDR_CID_ANY.

Ok. Then I'll only allow VMADDR_CID_HOST as the destination CID, since 
we don't support loopback mode.

> > > > +	/* The host's port range [MIN_HOST_EPHEMERAL_PORT, 0xFFFFFFFF)
> > > is
> > > > +	 * reserved as ephemeral ports, which are used as the host's ports
> > > > +	 * when the host initiates connections.
> > > > +	 */
> > > > +	if (port > MAX_HOST_LISTEN_PORT)
> > > > +		return false;
> > >
> > > Without this if statement the guest will attempt to connect.  I guess
> > > there will be no listen sockets above MAX_HOST_LISTEN_PORT, so the
> > > connection attempt will fail.
> >
> > You're correct.
> > To use the vsock common infrastructure, we have to map Hyper-V's
> > GUID <VM_ID, Service_ID> to int <cid, port>, and hence we must limit
> > the port range we can listen() on to [0, MAX_LISTEN_PORT], i.e.
> > we can only use half of the whole 32-bit port space for listen().
> > This is detailed in the long comments starting at about Line 100.
> >
> > > ...but hardcode this knowledge into the guest driver?
> > I'd like the guest's connect() to fail immediately here.
> > IMO this is better than a connect timeout. :-)
> 
> Thanks for explaining.  Perhaps the comment could be updated:
> 
>  /* The host's port range [MIN_HOST_EPHEMERAL_PORT, 0xFFFFFFFF) is
>   * reserved as ephemeral ports, which are used as the host's ports when
>   * the host initiates connections.
>   *
>   * Perform this check in the guest so an immediate error is produced
>   * instead of a timeout.
>   */
> 
> Stefan

Thank you, Stefan! 
Please see the below for the updated version of the function:

static bool hvs_stream_allow(u32 cid, u32 port)
{
        /* The host's port range [MIN_HOST_EPHEMERAL_PORT, 0xFFFFFFFF) is
         * reserved as ephemeral ports, which are used as the host's ports
         * when the host initiates connections.
         *
         * Perform this check in the guest so an immediate error is produced
         * instead of a timeout.
         */
        if (port > MAX_HOST_LISTEN_PORT)
                return false;

        if (cid == VMADDR_CID_HOST)
                return true;

        return false;
}

I'll send a v2 of the patch later today.

-- Dexuan