netdev - RE: [PATCH v2 net-next 1/1] 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] [day] [month] [year] [list]

Message-ID: <KL1P15301MB0008DD63FB6D476484DFF5BFBF9B0@KL1P15301MB0008.APCP153.PROD.OUTLOOK.COM>
Date:   Fri, 25 Aug 2017 03:19:22 +0000
From:   Dexuan Cui <decui@...rosoft.com>
To:     David Miller <davem@...emloft.net>
CC:     "jhansen@...are.com" <jhansen@...are.com>,
        "stefanha@...hat.com" <stefanha@...hat.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "mkubecek@...e.cz" <mkubecek@...e.cz>,
        "joe@...ches.com" <joe@...ches.com>,
        "olaf@...fle.de" <olaf@...fle.de>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        "jasowang@...hat.com" <jasowang@...hat.com>,
        "KY Srinivasan" <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        "dave.scott@...ker.com" <dave.scott@...ker.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "apw@...onical.com" <apw@...onical.com>,
        "rolf.neugebauer@...ker.com" <rolf.neugebauer@...ker.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "marcelo.cerri@...onical.com" <marcelo.cerri@...onical.com>,
        "devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
        "vkuznets@...hat.com" <vkuznets@...hat.com>,
        "georgezhang@...are.com" <georgezhang@...are.com>,
        "dan.carpenter@...cle.com" <dan.carpenter@...cle.com>,
        "acking@...are.com" <acking@...are.com>,
        "dtor@...are.com" <dtor@...are.com>,
        "grantr@...are.com" <grantr@...are.com>,
        "cavery@...hat.com" <cavery@...hat.com>
Subject: RE: [PATCH v2 net-next 1/1] hv_sock: implements Hyper-V transport for
 Virtual Sockets (AF_VSOCK)

> From: David Miller [mailto:davem@...emloft.net]
> Sent: Thursday, August 24, 2017 18:20
> > +#define VMBUS_PKT_TRAILER  (sizeof(u64))
>
> This is not the packet trailer, it's the size of the packet trailer.

Thanks! I'll change it to VMBUS_PKT_TRAILER_SIZE.

> > +   /* Have we sent the zero-length packet (FIN)? */
> > +   unsigned long fin_sent;
>
> Why does this need to be atomic?  Why can't a smaller simpler
It doesn't have to be. It was originally made for a quick workaround.
Thanks! I should do it in the right way now.

> mechanism be used to make sure hvs_shutdown() only performs
> hvs_send_data() call once on the channel?
I'll change "fin_sent" to bool, and avoid test_and_set_bit().
I'll add lock_sock/release_sock()  in hvs_shutdown() like this:

static int hvs_shutdown(struct vsock_sock *vsk, int mode)
{
 ...
       lock_sock(sk);

        hvs = vsk->trans;
        if (hvs->fin_sent)
                goto out;

        send_buf = (struct hvs_send_buf *)&hdr;
        (void)hvs_send_data(hvs->chan, send_buf, 0);

        hvs->fin_sent = true;
out:
        release_sock(sk);
        return 0;
}

> > +static inline bool is_valid_srv_id(const uuid_le *id)
> > +{
> > +   return !memcmp(&id->b[4], &srv_id_template.b[4], sizeof(uuid_le) -
> 4);
> > +}
>
> Do not use the inline function attribute in *.c code.  Let the
> compiler decide.

OK. Will remove all the inline usages.

> > +   *((u32 *)&h->vm_srv_id) = vsk->local_addr.svm_port;
> > +   *((u32 *)&h->host_srv_id) = vsk->remote_addr.svm_port;
>
> There has to be a better way to express this.
I may need to define a uinon here. Let me try it.

 > And if this is partially initializing vm_srv_id, at a minimum
> endianness needs to be taken into account.
I may need to use cpu_to_le32(). Let me check it.