Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20200128214356.GH30412@brightrain.aerifal.cx>
Date: Tue, 28 Jan 2020 16:43:56 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] cuserid: support invocation with a NULL pointer
 argument

On Tue, Jan 28, 2020 at 08:40:32PM +0100, Sören Tempel wrote:
> I did not manage to find a copy of IEEE 1003.1-1988 (the last POSIX
> version where cuserid was last standardized) the Single UNIX
> specification version 2 does state the following though [1]:
> 
> 	If s is a null pointer, this representation is generated in an
> 	area that may be static (and thus overwritten by subsequent
> 	calls to cuserid()), the address of which is returned.
> 
> Even though this a legacy function it would therefore be nice for musl
> to support usage with a NULL pointer. I ran into this on Alpine Linux
> when using cdparanoia [2] which uses cuserid like this and therefore
> caused a crash on my system.
> 
> [1]: https://pubs.opengroup.org/onlinepubs/7908799/xsh/cuserid.html
> [2]: https://xiph.org/paranoia/index.html

I'm not sure whether to adopt this or not, but thanks for posting on
the list for discussion. In any case it's something we should try to
get fixed in apps that are using it, since this is no longer portable
usage and is gratuitously thread-unsafe.

> ---
>  src/legacy/cuserid.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/legacy/cuserid.c b/src/legacy/cuserid.c
> index 4e78798d..19206ba4 100644
> --- a/src/legacy/cuserid.c
> +++ b/src/legacy/cuserid.c
> @@ -5,10 +5,12 @@
>  
>  char *cuserid(char *buf)
>  {
> +	static char *usridbuf[L_cuserid];

Surely the * there is misplaced; the following should not compile with
it there:

>  	struct passwd pw, *ppw;
>  	long pwb[256];
>  	if (getpwuid_r(geteuid(), &pw, (void *)pwb, sizeof pwb, &ppw))
>  		return 0;
> +	buf = (buf) ? buf : usridbuf;
>  	snprintf(buf, L_cuserid, "%s", pw.pw_name);
>  	return buf;
>  }

Also no need for parens around (buf); the more idiomatic way to write
this would be:

if (!buf) buf = usridbuf;

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.