svn commit: r241141 - in head: include/rpc lib/libc/rpc sys/rpc

[Bruce Evans]

On Tue, 2 Oct 2012, Pedro F. Giffuni wrote:

> Log:
>  RPC: Convert all uid and gid variables of the type uid_t and gid_t.
>
>  This matches what upstream (OpenSolaris) does.
>
>  Tested by:	David Wolfskill
>  Obtained from:	Bull GNU/Linux NFSv4 project (libtirpc)
>  MFC after:	3 days

This still assumes that uid_t and gid_t are precisely u_int, in stronger
ways than before.

> Modified: head/lib/libc/rpc/authunix_prot.c
> ==============================================================================
> --- head/lib/libc/rpc/authunix_prot.c	Tue Oct  2 18:38:05 2012	(r241140)
> +++ head/lib/libc/rpc/authunix_prot.c	Tue Oct  2 19:00:56 2012	(r241141)
> @@ -60,7 +60,7 @@ xdr_authunix_parms(xdrs, p)
> 	XDR *xdrs;
> 	struct authunix_parms *p;
> {
> -	int **paup_gids;
> +	gid_t **paup_gids;
>
> 	assert(xdrs != NULL);
> 	assert(p != NULL);
> @@ -69,8 +69,8 @@ xdr_authunix_parms(xdrs, p)
>
> 	if (xdr_u_long(xdrs, &(p->aup_time))
> 	    && xdr_string(xdrs, &(p->aup_machname), MAX_MACHINE_NAME)
> -	    && xdr_int(xdrs, &(p->aup_uid))
> -	    && xdr_int(xdrs, &(p->aup_gid))
> +	    && xdr_u_int(xdrs, &(p->aup_uid))
> +	    && xdr_u_int(xdrs, &(p->aup_gid))
> 	    && xdr_array(xdrs, (char **) paup_gids,
> 		    &(p->aup_len), NGRPS, sizeof(int), (xdrproc_t)xdr_int) ) {
> 		return (TRUE);
>

xdr doesn't support arbitrary types.  Here the very name of xdr_u_int()
indicates that it only works on u_int's.  Its second arg must be a
pointer to u_int (misspelled unsigned in the man page, so it doesn't
match the function name in a different, harmless way).  The arg used
to be a pointer to an int, and the call to xdr_int() used to match that.
The arg is now a pointer to a uid_t or gid_t, and the call to xdr_u_int()
only matches that accidentally.  (The types happen to be uint32_t, which
happens to be u_int.)

More careful code would select an xdr translation function based on
sizeof(uid_t) etc.

The above xdr_array() call takes a element size arg that is necessary
for stepping through the array, but isn't careful to use sizeof() on
the correct type.  It uses sizeof() on a hard-coded type, and you just
changed the element type without changing the hard-coded type.  It used
to match (int == int), but now doesn't (int != gid_t).  sizeof() should
be applied to objects and not types to get the object size right without
hard-coding its type.

The first type of type error should be detected at compile time.  The
second one (the hard-coded sizeof(int)) probably cannot be.  And there
is yet another new type error in the xdr_array() call.  It takes a
pointer to a translation function.  The function used to match the elemennt
type, but now doesn't (int != gid_t, and also int != underlying type
of gid_t == u_int).  The API requires casting the pointer to a generic
one using an obfuscated typedef, so the compiler cannot detect this
type mismatch at compile time (without breaking the API generally).

Bruce