[RFC PATCH 0/2] Add AF_VSOCK support to getnameinfo(3) and getaddrinfo(3)

[RFC PATCH 0/2] Add AF_VSOCK support to getnameinfo(3) and getaddrinfo(3)

[Stefan Hajnoczi]

The AF_VSOCK address family has been available in Linux since 3.9.  It allows
virtual machines and hypervisors to communicate over a zero-configuration
transport without Ethernet or IP.

Many existing programs use getnameinfo(3) and getaddrinfo(3).  Porting programs
to support AF_VSOCK is easy if the library functions can handle this address
family.  Without support in glibc each program needs to duplicate address
parsing code and it becomes harder to port programs.

In Patch 1 I have commented on use of the <linux/vm_sockets.h> header which
declares struct sockaddr_vm.  I'm not familiar with the process for syncing
Linux uapi headers with glibc headers.  What is necessary to use this header
file (or write an equivalent inside glibc)?

I used #ifdef AF_VSOCK since not all the sysdeps/ will have this macro defined.
Is there a nicer way of handling this?

Stefan Hajnoczi (2):
  getnameinfo: Add AF_VSOCK support
  getaddrinfo: Add AF_VSOCK support

 inet/getnameinfo.c          |  52 ++++++++++++++++++++++
 sysdeps/posix/getaddrinfo.c | 104 ++++++++++++++++++++++++++++++++++++++------
 2 files changed, 143 insertions(+), 13 deletions(-)

-- 
2.7.4

[RFC PATCH 1/2] getnameinfo: Add AF_VSOCK support

[Stefan Hajnoczi]

Allow getnameinfo(3) to understand AF_VSOCK sockaddrs.  There is no name
resolution for AF_VSOCK so it's just a matter of formatting the <cid,
port> fields.

The AF_VSOCK address family has been available in Linux since 3.9.  It
allows virtual machines and hypervisors to communicate over a
zero-configuration transport without Ethernet or IP.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 inet/getnameinfo.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/inet/getnameinfo.c b/inet/getnameinfo.c
index 283da55..b5bc10e 100644
--- a/inet/getnameinfo.c
+++ b/inet/getnameinfo.c
@@ -72,6 +72,13 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <libc-lock.h>
 #include <scratch_buffer.h>
 
+/* TODO This uapi kernel header is needed for struct sockaddr_vm.  Do I need to
+ * import/reimplement the header for glibc?
+ */
+#ifdef AF_VSOCK
+#include <linux/vm_sockets.h>
+#endif /* AF_VSOCK */
+
 #ifdef HAVE_LIBIDN
 # include <libidn/idna.h>
 extern int __idna_to_unicode_lzlz (const char *input, char **output,
@@ -412,6 +419,22 @@ gni_host_local (struct scratch_buffer *tmpbuf,
   return checked_copy (host, hostlen, "localhost");
 }
 
+#ifdef AF_VSOCK
+/* Convert AF_VSOCK socket address, host part.   */
+static int
+gni_host_vsock (struct scratch_buffer *tmpbuf,
+		const struct sockaddr *sa, socklen_t addrlen,
+		char *host, socklen_t hostlen, int flags)
+{
+  const struct sockaddr_vm *svm = (const struct sockaddr_vm *) sa;
+
+  if ((flags & NI_NAMEREQD) && !(flags & NI_NUMERICHOST))
+    return EAI_NONAME;
+
+  return CHECKED_SNPRINTF (host, hostlen, "%u", svm->svm_cid);
+}
+#endif /* AF_VSOCK */
+
 /* Convert the host part of an AF_LOCAK socket address.   */
 static int
 gni_host (struct scratch_buffer *tmpbuf,
@@ -427,6 +450,11 @@ gni_host (struct scratch_buffer *tmpbuf,
     case AF_LOCAL:
       return gni_host_local (tmpbuf, sa, addrlen, host, hostlen, flags);
 
+#ifdef AF_VSOCK
+    case AF_VSOCK:
+      return gni_host_vsock (tmpbuf, sa, addrlen, host, hostlen, flags);
+#endif /* AF_VSOCK */
+
     default:
       return EAI_FAMILY;
     }
@@ -479,6 +507,20 @@ gni_serv_local (struct scratch_buffer *tmpbuf,
     (serv, servlen, ((const struct sockaddr_un *) sa)->sun_path);
 }
 
+#ifdef AF_VSOCK
+/* Convert service to string, AF_VSOCK variant.  */
+static int
+gni_serv_vsock (struct scratch_buffer *tmpbuf,
+	       const struct sockaddr *sa, socklen_t addrlen,
+	       char *serv, socklen_t servlen, int flags)
+{
+  const struct sockaddr_vm *svm = (const struct sockaddr_vm *) sa;
+
+  return CHECKED_SNPRINTF (serv, servlen, "%u", svm->svm_port);
+}
+#endif /* AF_VSOCK */
+
+
 /* Convert service to string, dispatching to the implementations
    above.  */
 static int
@@ -493,6 +535,10 @@ gni_serv (struct scratch_buffer *tmpbuf,
       return gni_serv_inet (tmpbuf, sa, addrlen, serv, servlen, flags);
     case AF_LOCAL:
       return gni_serv_local (tmpbuf, sa, addrlen, serv, servlen, flags);
+#ifdef AF_VSOCK
+    case AF_VSOCK:
+      return gni_serv_vsock (tmpbuf, sa, addrlen, serv, servlen, flags);
+#endif /* AF_VSOCK */
     default:
       return EAI_FAMILY;
     }
@@ -530,6 +576,12 @@ getnameinfo (const struct sockaddr *sa, socklen_t addrlen, char *host,
       if (addrlen < sizeof (struct sockaddr_in6))
 	return EAI_FAMILY;
       break;
+#ifdef AF_VSOCK
+    case AF_VSOCK:
+      if (addrlen < sizeof (struct sockaddr_vm))
+	return EAI_FAMILY;
+      break;
+#endif /* AF_VSOCK */
     default:
       return EAI_FAMILY;
     }
-- 
2.7.4

[RFC PATCH 2/2] getaddrinfo: Add AF_VSOCK support

[Stefan Hajnoczi]

Allow getaddrinfo(3) to construct AF_VSOCK sockaddrs.  There is no name
resolution for AF_VSOCK so the <cid, port> fields are populated from
numeric values.

The AF_VSOCK address family has been available in Linux since 3.9.  It
allows virtual machines and hypervisors to communicate over a
zero-configuration transport without Ethernet or IP.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 sysdeps/posix/getaddrinfo.c | 104 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 91 insertions(+), 13 deletions(-)

diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 740e955..8e7abaa 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -83,6 +83,9 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <resolv/res_hconf.h>
 #include <scratch_buffer.h>
 #include <inet/net-internal.h>
+#ifdef AF_VSOCK
+#include <linux/vm_sockets.h>
+#endif /* AF_VSOCK */
 
 #ifdef HAVE_LIBIDN
 extern int __idna_to_ascii_lz (const char *input, char **output, int flags);
@@ -1238,6 +1241,75 @@ gaih_inet (const char *name, const struct gaih_service *service,
   return result;
 }
 
+#ifdef AF_VSOCK
+static int
+gaih_vsock (const char *name, const struct gaih_service *service,
+	    const struct addrinfo *req, struct addrinfo **pai,
+	    unsigned int *naddrs)
+{
+  struct addrinfo *ai;
+  struct sockaddr_vm *svm;
+  unsigned int svm_port = 0;
+  unsigned int svm_cid = 0;
+  char *canon = NULL;
+
+  if (req->ai_protocol != 0)
+    return -EAI_SOCKTYPE;
+
+  if (service != NULL)
+    {
+      if (service->num < 0)
+	  return -EAI_SERVICE;
+
+      svm_port = service->num;
+    }
+
+  if (name != NULL)
+    {
+      char *c;
+      svm_cid = strtoul (name, &c, 10);
+      if (c == name || *c != '\0')
+	  return EAI_NONAME;
+
+      if (req->ai_flags & AI_CANONNAME)
+	{
+	  canon = strdup (name);
+	  if (canon == NULL)
+	    return -EAI_MEMORY;
+	}
+    }
+
+  ai = *pai = malloc (sizeof (struct addrinfo) + sizeof (struct sockaddr_vm));
+  if (ai == NULL)
+    {
+      free (canon);
+      return -EAI_MEMORY;
+    }
+
+  ai->ai_flags = req->ai_flags;
+  ai->ai_family = req->ai_family;
+  ai->ai_socktype = req->ai_socktype;
+  ai->ai_protocol = req->ai_protocol;
+  ai->ai_addrlen = sizeof (*svm);
+  ai->ai_addr = (void *) (ai + 1);
+  ai->ai_canonname = canon;
+  ai->ai_next = NULL;
+
+#ifdef _HAVE_SA_LEN
+  ai->ai_addr->sa_len = sizeof (*svm);
+#endif /* _HAVE_SA_LEN */
+  ai->ai_addr->sa_family = req->ai_family;
+
+  svm = (struct sockaddr_vm *) ai->ai_addr;
+  svm->svm_port = svm_port;
+  svm->svm_cid = svm_cid;
+  memset(svm->svm_zero, '\0', sizeof (svm->svm_zero));
+
+  *naddrs = 1;
+  return 0;
+}
+#endif /* AF_VSOCK */
+
 
 struct sort_result
 {
@@ -2372,26 +2444,32 @@ getaddrinfo (const char *name, const char *service,
       scratch_buffer_init (&tmpbuf);
       last_i = gaih_inet (name, pservice, hints, end, &naddrs, &tmpbuf);
       scratch_buffer_free (&tmpbuf);
-
-      if (last_i != 0)
-	{
-	  freeaddrinfo (p);
-	  __free_in6ai (in6ai);
-
-	  return -last_i;
-	}
-      while (*end)
-	{
-	  end = &((*end)->ai_next);
-	  ++nresults;
-	}
     }
+#ifdef AF_VSOCK
+  else if (hints->ai_family == AF_VSOCK)
+    {
+      last_i = gaih_vsock (name, pservice, hints, end, &naddrs);
+    }
+#endif /* AF_VSOCK */
   else
     {
       __free_in6ai (in6ai);
       return EAI_FAMILY;
     }
 
+  if (last_i != 0)
+    {
+      freeaddrinfo (p);
+      __free_in6ai (in6ai);
+
+      return -last_i;
+    }
+  while (*end)
+    {
+      end = &((*end)->ai_next);
+      ++nresults;
+    }
+
   if (naddrs > 1)
     {
       /* Read the config file.  */
-- 
2.7.4

Re: [RFC PATCH 2/2] getaddrinfo: Add AF_VSOCK support

[Florian Weimer]

On 09/29/2016 12:27 PM, Stefan Hajnoczi wrote:
> +  ai = *pai = malloc (sizeof (struct addrinfo) + sizeof (struct sockaddr_vm));

You should define a local struct type for this, to make it more obvious 
that you got the alignment correct.

 > +  ai->ai_addr = (void *) (ai + 1);

Then you can avoid this pointer arithmetic as well.

If we decide to add this functionality, we'll need tests for it.

Florian

Re: [RFC PATCH 0/2] Add AF_VSOCK support to getnameinfo(3) and getaddrinfo(3)

[Florian Weimer]

* Stefan Hajnoczi:

> Many existing programs use getnameinfo(3) and getaddrinfo(3).
> Porting programs to support AF_VSOCK is easy if the library
> functions can handle this address family.  Without support in glibc
> each program needs to duplicate address parsing code and it becomes
> harder to port programs.

What has changed since the previous discussion?

  <https://sourceware.org/ml/libc-help/2015-08/msg00004.html>

How do you expect that applications will know that they have to pass
AF_VSOCK to getaddrinfo instead of AF_UNSPEC?

Re: [RFC PATCH 0/2] Add AF_VSOCK support to getnameinfo(3) and getaddrinfo(3)

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1433 bytes --]

On Thu, Sep 29, 2016 at 03:25:55PM +0200, Florian Weimer wrote:
> * Stefan Hajnoczi:
> 
> > Many existing programs use getnameinfo(3) and getaddrinfo(3).
> > Porting programs to support AF_VSOCK is easy if the library
> > functions can handle this address family.  Without support in glibc
> > each program needs to duplicate address parsing code and it becomes
> > harder to port programs.
> 
> What has changed since the previous discussion?
> 
>   <https://sourceware.org/ml/libc-help/2015-08/msg00004.html>
> 
> How do you expect that applications will know that they have to pass
> AF_VSOCK to getaddrinfo instead of AF_UNSPEC?

For example ncat(1) has --unixsock and --udp command-line options.  A
--vsock option can be added.  At that point the program knows to use
AF_VSOCK and the same getaddrinfo(3) code path can be used by TCP, UDP,
UNIX, and vsock.

The AF_UNSPEC approach where getaddrinfo(3) parses an arbitrary string
and figures out the address family can't be supported for the security
reasons you explained previously.  But the other getnameinfo(3) and
getaddrinfo(3) use cases still make sense and simplify porting existing
applications to AF_VSOCK.

One note about the previous discussion: I had proposed a [vsock:<cid>]
syntax for the host.  It's not implement in this patch but I will do it
for the next revision because it fits better into IPv4/IPv6 and URL
parsing.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [RFC PATCH 0/2] Add AF_VSOCK support to getnameinfo(3) and getaddrinfo(3)

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1968 bytes --]

On Fri, Sep 30, 2016 at 12:10:40PM +0100, Stefan Hajnoczi wrote:
> On Thu, Sep 29, 2016 at 03:25:55PM +0200, Florian Weimer wrote:
> > * Stefan Hajnoczi:
> > 
> > > Many existing programs use getnameinfo(3) and getaddrinfo(3).
> > > Porting programs to support AF_VSOCK is easy if the library
> > > functions can handle this address family.  Without support in glibc
> > > each program needs to duplicate address parsing code and it becomes
> > > harder to port programs.
> > 
> > What has changed since the previous discussion?
> > 
> >   <https://sourceware.org/ml/libc-help/2015-08/msg00004.html>
> > 
> > How do you expect that applications will know that they have to pass
> > AF_VSOCK to getaddrinfo instead of AF_UNSPEC?
> 
> For example ncat(1) has --unixsock and --udp command-line options.  A
> --vsock option can be added.  At that point the program knows to use
> AF_VSOCK and the same getaddrinfo(3) code path can be used by TCP, UDP,
> UNIX, and vsock.
> 
> The AF_UNSPEC approach where getaddrinfo(3) parses an arbitrary string
> and figures out the address family can't be supported for the security
> reasons you explained previously.  But the other getnameinfo(3) and
> getaddrinfo(3) use cases still make sense and simplify porting existing
> applications to AF_VSOCK.

Hi Florian,
Did this explanation make sense?  There are many applications that know
the address family but still want to use getaddrinfo(3) to construct a
sockaddr.

> One note about the previous discussion: I had proposed a [vsock:<cid>]
> syntax for the host.  It's not implement in this patch but I will do it
> for the next revision because it fits better into IPv4/IPv6 and URL
> parsing.

Looking into this more the [vsock:<cid>] style quoting is appropriate
for URIs but not for getnameinfo(3) where raw IPv6 are also produced
instead of block quoted addresses.  So I don't think it is necessary the
current formatting.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [RFC PATCH 0/2] Add AF_VSOCK support to getnameinfo(3) and getaddrinfo(3)

[Florian Weimer]

On 09/30/2016 01:10 PM, Stefan Hajnoczi wrote:
> On Thu, Sep 29, 2016 at 03:25:55PM +0200, Florian Weimer wrote:
>> * Stefan Hajnoczi:
>>
>>> Many existing programs use getnameinfo(3) and getaddrinfo(3).
>>> Porting programs to support AF_VSOCK is easy if the library
>>> functions can handle this address family.  Without support in glibc
>>> each program needs to duplicate address parsing code and it becomes
>>> harder to port programs.
>>
>> What has changed since the previous discussion?
>>
>>   <https://sourceware.org/ml/libc-help/2015-08/msg00004.html>
>>
>> How do you expect that applications will know that they have to pass
>> AF_VSOCK to getaddrinfo instead of AF_UNSPEC?
>
> For example ncat(1) has --unixsock and --udp command-line options.  A
> --vsock option can be added.  At that point the program knows to use
> AF_VSOCK and the same getaddrinfo(3) code path can be used by TCP, UDP,
> UNIX, and vsock.

ncat doesn't use getaddrinfo AFAICS, so this isn't going to help it. 
The larger nmap codebase has a call to getaddrinfo, but the code leading 
to it assumes 16-bit port numbers, so it won't be able to use 
getaddrinfo either.

Do you have a better example?

Is there a test guest system which has NFS over AF_VSOCK running?  It 
looks to me that AF_VSOCK is fundamentally insecure, and I'm not sure if 
it can be fixed.

Florian

Re: [RFC PATCH 0/2] Add AF_VSOCK support to getnameinfo(3) and getaddrinfo(3)

[Florian Weimer]

On 10/06/2016 01:19 PM, Florian Weimer wrote:

>>> How do you expect that applications will know that they have to pass
>>> AF_VSOCK to getaddrinfo instead of AF_UNSPEC?
>>
>> For example ncat(1) has --unixsock and --udp command-line options.  A
>> --vsock option can be added.  At that point the program knows to use
>> AF_VSOCK and the same getaddrinfo(3) code path can be used by TCP, UDP,
>> UNIX, and vsock.
>
> ncat doesn't use getaddrinfo AFAICS, so this isn't going to help it. The
> larger nmap codebase has a call to getaddrinfo, but the code leading to
> it assumes 16-bit port numbers, so it won't be able to use getaddrinfo
> either.
>
> Do you have a better example?

I've looked briefly at nfs-utils.  I don't see how you can patch in 
AF_VSOCK support using the proposed getaddrinfo support, either.  It 
seems to do the address and service lookup separately.

Thanks,
Florian

Re: [RFC PATCH 0/2] Add AF_VSOCK support to getnameinfo(3) and getaddrinfo(3)

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2013 bytes --]

On Fri, Oct 07, 2016 at 04:02:19PM +0200, Florian Weimer wrote:
> On 10/06/2016 01:19 PM, Florian Weimer wrote:
> 
> > > > How do you expect that applications will know that they have to pass
> > > > AF_VSOCK to getaddrinfo instead of AF_UNSPEC?
> > > 
> > > For example ncat(1) has --unixsock and --udp command-line options.  A
> > > --vsock option can be added.  At that point the program knows to use
> > > AF_VSOCK and the same getaddrinfo(3) code path can be used by TCP, UDP,
> > > UNIX, and vsock.
> > 
> > ncat doesn't use getaddrinfo AFAICS, so this isn't going to help it. The
> > larger nmap codebase has a call to getaddrinfo, but the code leading to
> > it assumes 16-bit port numbers, so it won't be able to use getaddrinfo
> > either.
> > 
> > Do you have a better example?
> 
> I've looked briefly at nfs-utils.  I don't see how you can patch in AF_VSOCK
> support using the proposed getaddrinfo support, either.  It seems to do the
> address and service lookup separately.

There are quite a few code paths in mount.nfs(8), maybe you are looking
at an unrelated one?  The nfs-utils patch series I've proposed does not
use service lookup:

https://www.spinics.net/lists/linux-nfs/msg60291.html

  # mount.nfs 2:/export /mnt -v -o clientaddr=3,proto=vsock

(Most code paths are not relevant to NFS over AF_VSOCK, which only
supports NFSv4+ and therefore no UDP or no rpcbind/portmap.)

The port=NUM option is passed to mount(2) as a string.  See
nfs_parse_mount_options() in the kernel for parsing code.


One of the factors that makes me think glibc should support AF_VSOCK in
getaddrinfo(3) is that programs that pass around struct addrinfo are
hard to modify.  Only glibc can allocate struct addrinfo because only
glibc knows how to free addrinfo in freeaddrinfo().

It's not possible to skip getaddrinfo(3) for AF_VSOCK and allocate
struct addrinfo manually since the details of the struct addrinfo memory
allocation are private to glibc.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [RFC PATCH 0/2] Add AF_VSOCK support to getnameinfo(3) and getaddrinfo(3)

[Florian Weimer]

On 12/05/2016 04:58 PM, Stefan Hajnoczi wrote:
> On Fri, Oct 07, 2016 at 04:02:19PM +0200, Florian Weimer wrote:
>> On 10/06/2016 01:19 PM, Florian Weimer wrote:
>>
>>>>> How do you expect that applications will know that they have to pass
>>>>> AF_VSOCK to getaddrinfo instead of AF_UNSPEC?
>>>>
>>>> For example ncat(1) has --unixsock and --udp command-line options.  A
>>>> --vsock option can be added.  At that point the program knows to use
>>>> AF_VSOCK and the same getaddrinfo(3) code path can be used by TCP, UDP,
>>>> UNIX, and vsock.
>>>
>>> ncat doesn't use getaddrinfo AFAICS, so this isn't going to help it. The
>>> larger nmap codebase has a call to getaddrinfo, but the code leading to
>>> it assumes 16-bit port numbers, so it won't be able to use getaddrinfo
>>> either.
>>>
>>> Do you have a better example?
>>
>> I've looked briefly at nfs-utils.  I don't see how you can patch in AF_VSOCK
>> support using the proposed getaddrinfo support, either.  It seems to do the
>> address and service lookup separately.
>
> There are quite a few code paths in mount.nfs(8), maybe you are looking
> at an unrelated one?  The nfs-utils patch series I've proposed does not
> use service lookup:
>
> https://www.spinics.net/lists/linux-nfs/msg60291.html
>
>   # mount.nfs 2:/export /mnt -v -o clientaddr=3,proto=vsock

Well, that was my point.

You claimed that we need to add this to getaddrinfo to simplify porting 
sockets code to AF_VSOCK.  But based on my analysis of the examples you 
provided, using getaddrinfo would not simplify matters at all or 
introduce bugs (due to truncation of port numbers to 16 bits).

> One of the factors that makes me think glibc should support AF_VSOCK in
> getaddrinfo(3) is that programs that pass around struct addrinfo are
> hard to modify.  Only glibc can allocate struct addrinfo because only
> glibc knows how to free addrinfo in freeaddrinfo().

That's certainly valid.  But then, I don't see any programs which use 
getaddrinfo and could transparently upgrade to AF_VSOCK anyway.

Thanks,
Florian

Re: [RFC PATCH 0/2] Add AF_VSOCK support to getnameinfo(3) and getaddrinfo(3)

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2644 bytes --]

On Mon, Dec 05, 2016 at 05:02:35PM +0100, Florian Weimer wrote:
> On 12/05/2016 04:58 PM, Stefan Hajnoczi wrote:
> > On Fri, Oct 07, 2016 at 04:02:19PM +0200, Florian Weimer wrote:
> > > On 10/06/2016 01:19 PM, Florian Weimer wrote:
> > > 
> > > > > > How do you expect that applications will know that they have to pass
> > > > > > AF_VSOCK to getaddrinfo instead of AF_UNSPEC?
> > > > > 
> > > > > For example ncat(1) has --unixsock and --udp command-line options.  A
> > > > > --vsock option can be added.  At that point the program knows to use
> > > > > AF_VSOCK and the same getaddrinfo(3) code path can be used by TCP, UDP,
> > > > > UNIX, and vsock.
> > > > 
> > > > ncat doesn't use getaddrinfo AFAICS, so this isn't going to help it. The
> > > > larger nmap codebase has a call to getaddrinfo, but the code leading to
> > > > it assumes 16-bit port numbers, so it won't be able to use getaddrinfo
> > > > either.
> > > > 
> > > > Do you have a better example?
> > > 
> > > I've looked briefly at nfs-utils.  I don't see how you can patch in AF_VSOCK
> > > support using the proposed getaddrinfo support, either.  It seems to do the
> > > address and service lookup separately.
> > 
> > There are quite a few code paths in mount.nfs(8), maybe you are looking
> > at an unrelated one?  The nfs-utils patch series I've proposed does not
> > use service lookup:
> > 
> > https://www.spinics.net/lists/linux-nfs/msg60291.html
> > 
> >   # mount.nfs 2:/export /mnt -v -o clientaddr=3,proto=vsock
> 
> Well, that was my point.
> 
> You claimed that we need to add this to getaddrinfo to simplify porting
> sockets code to AF_VSOCK.  But based on my analysis of the examples you
> provided, using getaddrinfo would not simplify matters at all or introduce
> bugs (due to truncation of port numbers to 16 bits).

getaddrinfo(3) is used by mount.nfs(8) to go from string to sockaddr
(service lookup isn't used though).  Due to the freeaddrinfo(3) issue I
explained below it's much simpler to have getaddrinfo(3) support than to
modify the program to treat struct addrinfo specially for AF_VSOCK only.

You disagree that getaddrinfo(3) support simplifies here?

> > One of the factors that makes me think glibc should support AF_VSOCK in
> > getaddrinfo(3) is that programs that pass around struct addrinfo are
> > hard to modify.  Only glibc can allocate struct addrinfo because only
> > glibc knows how to free addrinfo in freeaddrinfo().
> 
> That's certainly valid.  But then, I don't see any programs which use
> getaddrinfo and could transparently upgrade to AF_VSOCK anyway.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [RFC PATCH 0/2] Add AF_VSOCK support to getnameinfo(3) and getaddrinfo(3)

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1938 bytes --]

On Thu, Oct 06, 2016 at 01:19:00PM +0200, Florian Weimer wrote:
> On 09/30/2016 01:10 PM, Stefan Hajnoczi wrote:
> > On Thu, Sep 29, 2016 at 03:25:55PM +0200, Florian Weimer wrote:
> > > * Stefan Hajnoczi:
> > > 
> > > > Many existing programs use getnameinfo(3) and getaddrinfo(3).
> > > > Porting programs to support AF_VSOCK is easy if the library
> > > > functions can handle this address family.  Without support in glibc
> > > > each program needs to duplicate address parsing code and it becomes
> > > > harder to port programs.
> > > 
> > > What has changed since the previous discussion?
> > > 
> > >   <https://sourceware.org/ml/libc-help/2015-08/msg00004.html>
> > > 
> > > How do you expect that applications will know that they have to pass
> > > AF_VSOCK to getaddrinfo instead of AF_UNSPEC?
> > 
> > For example ncat(1) has --unixsock and --udp command-line options.  A
> > --vsock option can be added.  At that point the program knows to use
> > AF_VSOCK and the same getaddrinfo(3) code path can be used by TCP, UDP,
> > UNIX, and vsock.
> 
> ncat doesn't use getaddrinfo AFAICS, so this isn't going to help it. The
> larger nmap codebase has a call to getaddrinfo, but the code leading to it
> assumes 16-bit port numbers, so it won't be able to use getaddrinfo either.
> 
> Do you have a better example?

I just sent nfs-utils patches for mount.nfs(8):
https://marc.info/?l=linux-nfs&m=147583470431780&w=2

The nfs-utils code passes around a struct addrinfo pointer obtained by
getaddrinfo(3) from nfs_try_mount().

If getaddrinfo(3) supports .ai_family = AF_VSOCK then it's easy to reuse
the existing code without adding special cases for AF_VSOCK.

> Is there a test guest system which has NFS over AF_VSOCK running?

Sure, I can set something up for you.

> It looks
> to me that AF_VSOCK is fundamentally insecure, and I'm not sure if it can be
> fixed.

In what way?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]