Re: [PATCH,HURD] hurdsock: reject negative domains

[Pino Toscano]

Alle mercoledì 23 novembre 2011, Thomas Schwinge ha scritto:
> Hi!
> 
> On Mon, 21 Nov 2011 22:23:26 +0100, Pino Toscano 
<toscano.pino@tiscali.it> wrote:
> > Reject negative socket domains right away; otherwise, it is
> > possible to read and set out-of-bounds locations of the `servers'
> > array (returning the values at those invalid memory locations),
> > and even try to deallocate ports with random values if the `dead'
> > parameter is different than zero.
> > 
> > 2011-11-21  Pino Toscano  <toscano.pino@tiscali.it>
> > 
> >     * hurd/hurdsock.c (_hurd_socket_server): Check for negative
> >     domains, and reject them.
> > 
> > --- a/hurd/hurdsock.c
> > +++ b/hurd/hurdsock.c
> > @@ -47,6 +47,12 @@ _hurd_socket_server (int domain, int dea
> > 
> >  {
> >  
> >    socket_t server;
> > 
> > +  if (domain < 0)
> > +    {
> > +      errno = EAFNOSUPPORT;
> > +      return MACH_PORT_NULL;
> > +    }
> > +
> 
> Thanks; the issue is valid, but we may want to fix it differently:
> _hurd_socket_server is an internal function, and internally we should
> always know what we're doing: that is, should only be calling it with
> valid data, such as the PF_* constants -- which is done in all places
> but socket and socketpair, which happen to be external interfaces. 
> Should instead in these two functions the domain parameter be
> checked for validity (and negative ones refused with EINVAL)?

On the other hand, the whole logic of the port cache as array (which 
make negative indexes not acceptable) is entirely in hurdsock.c, so IMHO 
it wouldn't make sense to know that (internal) logic also outide that 
file.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.