Re: [PATCH] getaddrinfo: return EAI_SERVICE when TCP or UDP port out of 1-65535 range

[Pavel Simerda <psimerda at redhat dot com>]

----- Original Message -----
> From: "Pavel Simerda" <psimerda@redhat.com>
> To: "Shawn Landden" <shawn@churchofgit.com>
> Cc: libc-alpha@sourceware.org, "Siddhesh Poyarekar" <siddhesh@redhat.com>, "Carlos O'Donell" <codonell@redhat.com>
> Sent: Wednesday, December 4, 2013 5:14:40 PM
> Subject: Re: [PATCH] getaddrinfo: return EAI_SERVICE when TCP or UDP port out of 1-65535 range
> 
> ----- Original Message -----
> > From: "Shawn Landden" <shawn@churchofgit.com>
> > To: libc-alpha@sourceware.org
> > Cc: "Shawn Landden" <shawn@churchofgit.com>
> > Sent: Wednesday, December 4, 2013 4:02:34 PM
> > Subject: [PATCH] getaddrinfo: return EAI_SERVICE when TCP or UDP port out
> > of 1-65535 range
> 
> The patch doesn't look entirely right:
> 
> 1) Existing application might use service="0" if they want to fill in the
> port later for some reason. And I personally think it's a valid use case as
> the behavior of service="0" shouldn't IMO be the same as of service=NULL. I
> propose to leave out the zero check.
> 
> See: https://sourceware.org/bugzilla/show_bug.cgi?id=14990
> 
> 2) There are other values of socktype than the enumerated ones including
> zero. I propose to avoid checking the socktype.

3) AF_UNSPEC basically means AF_INET+AF_INET6, so it shouldn't be excluded. That said, the check could be moved just before the line where gaih_inet() is called from getaddrinfo(), or possibly even inside gaih_inet(). There you would only need to check the upper bound of the port.

But then it might be indeed better to wait for my patch and see what can be done.

Cheers,

Pavel

> Also, I was maybe waiting too long with my patch fixing the bug I mentioned
> above and cleaning up the service/servname stuff. I'll post it ASAP and I'll
> include the port upper bound check. In the meantime, I don't have any strong
> preference as whether to add the upper bound check to the current code or
> not, I would list like to have #1 and #2 taken into consideration.
> 
> Cheers,
> 
> Pavel
> 
> 
> > Reported-by: Martin Pool <mbp@sourcefrog.net>
> > Signed-off-by: Shawn Landden <shawn@churchofgit.com>
> > ---
> >  sysdeps/posix/getaddrinfo.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
> > index 8218237..5b3b10d 100644
> > --- a/sysdeps/posix/getaddrinfo.c
> > +++ b/sysdeps/posix/getaddrinfo.c
> > @@ -2391,6 +2391,12 @@ getaddrinfo (const char *name, const char *service,
> >  
> >  	  gaih_service.num = -1;
> >  	}
> > +      else if ((hints->ai_family == AF_INET || hints->ai_family ==
> > AF_INET6)
> > +               && (hints->ai_socktype == SOCK_STREAM || hints->ai_socktype
> > == SOCK_DGRAM)
> > +               && (gaih_service.num == 0 || gaih_service.num > 65535))
> > +        {
> > +          return EAI_SERVICE;
> > +        }
> >  
> >        pservice = &gaih_service;
> >      }
> > --
> > 1.8.5
> > 
> > 
> 
>