Re: [PATCH] libtirpc: handle large numbers of supplimental groups gracefully

[Chuck Lever <chuck.lever@oracle.com>]

See comments in RH bz 565507.

On 02/16/2010 11:12 AM, Jeff Layton wrote:
> If authunix_create_default() is called by a user with more than 16
> supplimental groups, it'll abort(), which causes the program to
> crash and coredump.
>
> Fix it to handle this situation gracefully. Get the number of
> groups that the user has first, and then allocate a big enough
> buffer to hold them. Then, just don't let the lower function use
> more than the NGRPS groups.
>
> Also fix up the error handling in this function so that it just
> returns a NULL pointer on error and logs a message via warnx()
> instead of calling abort().
>
> Signed-off-by: Jeff Layton<jlayton@redhat.com>
> ---
>   src/auth_unix.c |   61 +++++++++++++++++++++++++++++++++++++++++++++++++-----
>   1 files changed, 55 insertions(+), 6 deletions(-)
>
> diff --git a/src/auth_unix.c b/src/auth_unix.c
> index 71ca15d..4ac1263 100644
> --- a/src/auth_unix.c
> +++ b/src/auth_unix.c
> @@ -49,6 +49,7 @@
>   #include<stdlib.h>
>   #include<unistd.h>
>   #include<string.h>
> +#include<errno.h>
>
>   #include<rpc/types.h>
>   #include<rpc/xdr.h>
> @@ -175,20 +176,68 @@ AUTH *
>   authunix_create_default()
>   {
>   	int len;
> +	size_t bufsize = 0;
>   	char machname[MAXHOSTNAMELEN + 1];
>   	uid_t uid;
>   	gid_t gid;
> -	gid_t gids[NGRPS];
> +	gid_t *gids = NULL;
> +	AUTH *auth;
> +
> +	if (gethostname(machname, sizeof machname) == -1) {
> +		warnx("%s: gethostname() failed: %s", __func__,
> +			strerror(errno));
> +		return NULL;
> +	}
>
> -	if (gethostname(machname, sizeof machname) == -1)
> -		abort();
>   	machname[sizeof(machname) - 1] = 0;
>   	uid = geteuid();
>   	gid = getegid();
> -	if ((len = getgroups(NGRPS, gids))<  0)
> -		abort();
> +
> +retry:
> +	if ((len = getgroups(0, NULL))<  0) {
> +		warnx("%s: failed to get number of groups: %s", __func__,
> +			strerror(errno));
> +		return NULL;
> +	}
> +
> +	/* paranoia -- never pass a buffer smaller than NGRPS */
> +	bufsize = (len<  NGRPS ? NGRPS : len) * sizeof(gid_t);
> +	gids = mem_alloc(bufsize);
> +	if (gids == NULL) {
> +		warnx("%s: memory allocation failed: %s", __func__,
> +			strerror(ENOMEM));
> +		return gids;
> +	}
> +
> +	if (len == 0)
> +		goto no_groups;
> +
> +	len = getgroups(len, gids);
> +	if (len<  0) {
> +		mem_free(gids, bufsize);
> +		/*
> +		 * glibc mentions that it's possible for the number of groups
> +		 * to change between two getgroups calls. If that happens,
> +		 * retry the whole thing again.
> +		 */
> +		if (len == -EINVAL) {
> +			gids = NULL;
> +			bufsize = 0;
> +			goto retry;
> +		}
> +		warnx("%s: failed to get group list: %s", __func__,
> +			strerror(errno));
> +		return NULL;
> +	}
> +
> +	if (len>  NGRPS)
> +		len = NGRPS;
> +
> +no_groups:
>   	/* XXX: interface problem; those should all have been unsigned */
> -	return (authunix_create(machname, uid, gid, len, gids));
> +	auth = authunix_create(machname, uid, gid, len, gids);
> +	mem_free(gids, bufsize);
> +	return auth;
>   }
>
>   /*


-- 
chuck[dot]lever[at]oracle[dot]com