Re: [PATCH] fix errno not being set to ERANGE by getgr, getpw, and getspnam

[Rich Felker <dalias@libc.org>]

On Tue, Jun 06, 2017 at 12:21:39PM -0400, Rudolph Pereira wrote:
> Hi,
> 
> currently, the getgr, getpw, and getspnam functions in musl return ERANGE
> when the allocated buffer is not large enough, but do not set errno to the
> same value. This causes issues with utilities, for example the "shadow"
> utilities (useradd/mod, groupmod, etc.) which assume this behaviour (which
> at least gnu libc exhibits) and leads to groups having a small limit on the
> number of members.
> 
> The attached patch, against 1.1.16, corrects this.
> 
> Cheers,
> Rudolph

> --- musl-1.1.16.orig/src/passwd/getgr_r.c
> +++ musl-1.1.16/src/passwd/getgr_r.c
> @@ -1,5 +1,6 @@
>  #include "pwf.h"
>  #include <pthread.h>
> +#include <errno.h>
>  
>  #define FIX(x) (gr->gr_##x = gr->gr_##x-line+buf)
>  
> @@ -19,6 +20,7 @@
>  	if (*res && size < len + (nmem+1)*sizeof(char *) + 32) {
>  		*res = 0;
>  		rv = ERANGE;
> +		errno = ERANGE;
>  	}
>  	if (*res) {
>  		buf += (16-(uintptr_t)buf)%16;
> --- musl-1.1.16.orig/src/passwd/getpw_r.c
> +++ musl-1.1.16/src/passwd/getpw_r.c
> @@ -1,5 +1,6 @@
>  #include "pwf.h"
>  #include <pthread.h>
> +#include <errno.h>
>  
>  #define FIX(x) (pw->pw_##x = pw->pw_##x-line+buf)
>  
> @@ -16,6 +17,7 @@
>  	if (*res && size < len) {
>  		*res = 0;
>  		rv = ERANGE;
> +		errno = ERANGE;
>  	}
>  	if (*res) {
>  		memcpy(buf, line, len);
> --- musl-1.1.16.orig/src/passwd/getspnam_r.c
> +++ musl-1.1.16/src/passwd/getspnam_r.c
> @@ -3,6 +3,7 @@
>  #include <sys/stat.h>
>  #include <ctype.h>
>  #include <pthread.h>
> +#include <errno.h>
>  #include "pwf.h"
>  
>  /* This implementation support Openwall-style TCB passwords in place of
> @@ -104,6 +105,7 @@
>  		}
>  		if (buf[k-1] != '\n') {
>  			rv = ERANGE;
> +			errno = ERANGE;
>  			break;
>  		}

I don't think this patch is complete. A nonzero value of rv can also
come from __getpw_a/__getgr_a. Insted, just before return there should
probably be:

	if (rv) errno = rv;

Does that look correct? I'm not sure about getspnam_r; it might
actually be missing some error cases right now.

Rich