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
