bug-hurd
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Implement getsockopt (fd, SOL_SOCKET, SO_TYPE, ...)


From: Carl Fredrik Hammar
Subject: Re: [PATCH] Implement getsockopt (fd, SOL_SOCKET, SO_TYPE, ...)
Date: Thu, 15 Jul 2010 20:46:30 +0200
User-agent: Mutt/1.5.20 (2009-06-14)

Hi,

On Thu, Jul 15, 2010 at 05:15:25PM +0200, Emilio Pozuelo Monfort wrote:
> On 15/07/10 17:05, Emilio Pozuelo Monfort wrote:
> > On 15/07/10 16:49, Emilio Pozuelo Monfort wrote:
> >> Here it goes, with a good commit message:
> > 
> > Forgot to add the file before committing :(
> 
> Also check that the buffer is big enough.

According to POSIX this should be handled by silently truncating the
returned value.  See second paragraph of:
http://www.opengroup.org/onlinepubs/9699919799/functions/getsockopt.html

This also seems to be what pfinet does:
pfinet/linux-src/net/ipv4/tcp.c:tcp_getsockopt

>  error_t
>  S_socket_getopt (struct sock_user *user,
>                int level, int opt,
>                char **value, size_t *value_len)
>  {

The first thing you should do in any RPC that is not a stub is:
  if (!user)
    return EOPNOTSUPP;

Also I think you need to lock the user->sock->lock.

> +  switch (level)
> +    {
> +    case SOL_SOCKET:
> +      switch (opt)
> +     {
> +     case SO_TYPE:
> +       if (value_len == NULL || value == NULL || *value == NULL
> +         || value_len < sizeof (user->sock->pipe_class->sock_type))
> +         return EINVAL;

Beyond the handling size as described in POSIX, you don't need to test
any tests for NULL because pointers doesn't come from the client itself
but from MIG which gives you pointers into the reply message buffer.
In fact, even *value_len isn't passed from the client it's the size of
the buffer in the message which is hard coded to 2048 at the moment.
The code in pfinet doesn't do any checks either and you can even see
this directly by looking at hurd/RPC_socket_getopt.c in a glibc build
and pflocal/socketUser.c in a Hurd build.

So it seems you don't need to worry about size at all but I'd put in an
assert(*value_len >= sizeof(int)) just to be safe, and perhaps comments
for the pointers never being NULL.  You still need to set *value_len
though.

Also you might as well just use sizeof (int) since this is specified
by POSIX:
http://www.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_10_16

> +       *(int*)*value = user->sock->pipe_class->sock_type;
> +       *value_len = sizeof (user->sock->pipe_class->sock_type);
> +       return 0;
> +     default:
> +       break;
> +     }
> +    default:
> +      break;
> +    }
> +
>    return EOPNOTSUPP;
>  }

I think you should return ENOPROTOOPT here.  Don't forget to unlock.  ;-)

Regards,
  Fredrik



reply via email to

[Prev in Thread] Current Thread [Next in Thread]