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]
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.

Powered by blists - more mailing lists