* [musl] [PATCH v2] cuserid: support invocation with a NULL pointer argument @ 2020-01-29 11:20 Sören Tempel 2020-01-29 11:59 ` Sören Tempel 0 siblings, 1 reply; 5+ messages in thread From: Sören Tempel @ 2020-01-29 11:20 UTC (permalink / raw) To: musl 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 --- src/legacy/cuserid.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/legacy/cuserid.c b/src/legacy/cuserid.c index 4e78798d..fd7832e4 100644 --- a/src/legacy/cuserid.c +++ b/src/legacy/cuserid.c @@ -5,10 +5,12 @@ char *cuserid(char *buf) { + static char usridbuf[L_cuserid]; struct passwd pw, *ppw; long pwb[256]; if (getpwuid_r(geteuid(), &pw, (void *)pwb, sizeof pwb, &ppw)) return 0; + if (!buf) buf = usridbuf; snprintf(buf, L_cuserid, "%s", pw.pw_name); return buf; } ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [musl] [PATCH v2] cuserid: support invocation with a NULL pointer argument 2020-01-29 11:20 [musl] [PATCH v2] cuserid: support invocation with a NULL pointer argument Sören Tempel @ 2020-01-29 11:59 ` Sören Tempel 2020-03-22 9:51 ` Sören Tempel 0 siblings, 1 reply; 5+ messages in thread From: Sören Tempel @ 2020-01-29 11:59 UTC (permalink / raw) To: musl Sorry, forgot to confirm my list subscription before sending the first version of this patch. Just wanted to reply to a comment on the previous version: On Tue, 28 Jan 2020, Rich Felker wrote: > 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. From my personal Alpine Linux packaging perspective: I would prefer either removing the function entirely or supporting the invocation with a NULL pointer argument. Currently, apps using this with a NULL pointer (e.g. cdparanoia) will crash at runtime which makes it more difficult to identify them. If the function would be removed entirely we would be able to detect, during compilation, which apps use it and patch them accordingly. Cheers, Sören ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [musl] [PATCH v2] cuserid: support invocation with a NULL pointer argument 2020-01-29 11:59 ` Sören Tempel @ 2020-03-22 9:51 ` Sören Tempel 2020-03-22 15:05 ` Rich Felker 0 siblings, 1 reply; 5+ messages in thread From: Sören Tempel @ 2020-03-22 9:51 UTC (permalink / raw) To: musl Sören Tempel <soeren@soeren-tempel.net> wrote: > From my personal Alpine Linux packaging perspective: I would prefer > either removing the function entirely or supporting the invocation with > a NULL pointer argument. Currently, apps using this with a NULL pointer > (e.g. cdparanoia) will crash at runtime which makes it more difficult to > identify them. If the function would be removed entirely we would be > able to detect, during compilation, which apps use it and patch them > accordingly. Ping. Greetings, Sören ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [musl] [PATCH v2] cuserid: support invocation with a NULL pointer argument 2020-03-22 9:51 ` Sören Tempel @ 2020-03-22 15:05 ` Rich Felker 2021-02-13 19:06 ` Rich Felker 0 siblings, 1 reply; 5+ messages in thread From: Rich Felker @ 2020-03-22 15:05 UTC (permalink / raw) To: musl On Sun, Mar 22, 2020 at 10:51:56AM +0100, Sören Tempel wrote: > Sören Tempel <soeren@soeren-tempel.net> wrote: > > From my personal Alpine Linux packaging perspective: I would prefer > > either removing the function entirely or supporting the invocation with > > a NULL pointer argument. Currently, apps using this with a NULL pointer > > (e.g. cdparanoia) will crash at runtime which makes it more difficult to > > identify them. If the function would be removed entirely we would be > > able to detect, during compilation, which apps use it and patch them > > accordingly. > > Ping. Thanks. I'm trying to catch up on patches and I just confirmed your research, so I think the patch is correct. I'll apply soon. Rich ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [musl] [PATCH v2] cuserid: support invocation with a NULL pointer argument 2020-03-22 15:05 ` Rich Felker @ 2021-02-13 19:06 ` Rich Felker 0 siblings, 0 replies; 5+ messages in thread From: Rich Felker @ 2021-02-13 19:06 UTC (permalink / raw) To: musl [-- Attachment #1: Type: text/plain, Size: 1039 bytes --] On Sun, Mar 22, 2020 at 11:05:57AM -0400, Rich Felker wrote: > On Sun, Mar 22, 2020 at 10:51:56AM +0100, Sören Tempel wrote: > > Sören Tempel <soeren@soeren-tempel.net> wrote: > > > From my personal Alpine Linux packaging perspective: I would prefer > > > either removing the function entirely or supporting the invocation with > > > a NULL pointer argument. Currently, apps using this with a NULL pointer > > > (e.g. cdparanoia) will crash at runtime which makes it more difficult to > > > identify them. If the function would be removed entirely we would be > > > able to detect, during compilation, which apps use it and patch them > > > accordingly. > > > > Ping. > > Thanks. I'm trying to catch up on patches and I just confirmed your > research, so I think the patch is correct. I'll apply soon. Sorry for taking so long to get back to this. The function seems to have been significantly broken on top of not supporting a null argument. See attached three patches which I plan to push if there are no stupid bugs in them. Rich [-- Attachment #2: 0001-cuserid-don-t-return-truncated-results.patch --] [-- Type: text/plain, Size: 986 bytes --] From a75283d777ed1827ed247dbb465818a0ce371c8f Mon Sep 17 00:00:00 2001 From: Rich Felker <dalias@aerifal.cx> Date: Sat, 13 Feb 2021 13:54:00 -0500 Subject: [PATCH 1/3] cuserid: don't return truncated results checking the length also drops the need to pull in snprintf. --- src/legacy/cuserid.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/legacy/cuserid.c b/src/legacy/cuserid.c index fd7832e4..3ff115a1 100644 --- a/src/legacy/cuserid.c +++ b/src/legacy/cuserid.c @@ -2,6 +2,7 @@ #include <pwd.h> #include <stdio.h> #include <unistd.h> +#include <string.h> char *cuserid(char *buf) { @@ -10,7 +11,10 @@ char *cuserid(char *buf) long pwb[256]; if (getpwuid_r(geteuid(), &pw, (void *)pwb, sizeof pwb, &ppw)) return 0; + size_t len = strnlen(pw.pw_name, L_cuserid); + if (len == L_cuserid) + return 0; if (!buf) buf = usridbuf; - snprintf(buf, L_cuserid, "%s", pw.pw_name); + memcpy(buf, pw.pw_name, len+1); return buf; } -- 2.21.0 [-- Attachment #3: 0002-fix-misuse-of-getpwuid_r-in-cuserid.patch --] [-- Type: text/plain, Size: 993 bytes --] From cc577d0e058b53df5c0fe2ac17890a41d77e94c5 Mon Sep 17 00:00:00 2001 From: Rich Felker <dalias@aerifal.cx> Date: Sat, 13 Feb 2021 13:59:44 -0500 Subject: [PATCH 2/3] fix misuse of getpwuid_r in cuserid getpwuid_r can return 0 but without a result in the case where there was no error but no record exists. in that case cuserid was treating it as success and copying junk out of pw.pw_name to the output buffer. --- src/legacy/cuserid.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/legacy/cuserid.c b/src/legacy/cuserid.c index 3ff115a1..07866acf 100644 --- a/src/legacy/cuserid.c +++ b/src/legacy/cuserid.c @@ -9,7 +9,8 @@ char *cuserid(char *buf) static char usridbuf[L_cuserid]; struct passwd pw, *ppw; long pwb[256]; - if (getpwuid_r(geteuid(), &pw, (void *)pwb, sizeof pwb, &ppw)) + getpwuid_r(geteuid(), &pw, (void *)pwb, sizeof pwb, &ppw); + if (!ppw) return 0; size_t len = strnlen(pw.pw_name, L_cuserid); if (len == L_cuserid) -- 2.21.0 [-- Attachment #4: 0003-fix-error-return-value-for-cuserid.patch --] [-- Type: text/plain, Size: 1100 bytes --] From 49b6df3d9f3645de55607f1ac60095b22661b334 Mon Sep 17 00:00:00 2001 From: Rich Felker <dalias@aerifal.cx> Date: Sat, 13 Feb 2021 14:03:23 -0500 Subject: [PATCH 3/3] fix error return value for cuserid the historical function was specified to return an empty string in the caller-provided buffer, not a null pointer, to indicate error when the argument is non-null. only when the argument is null should it return a null pointer on error. --- src/legacy/cuserid.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/legacy/cuserid.c b/src/legacy/cuserid.c index 07866acf..dcaf73d4 100644 --- a/src/legacy/cuserid.c +++ b/src/legacy/cuserid.c @@ -9,12 +9,13 @@ char *cuserid(char *buf) static char usridbuf[L_cuserid]; struct passwd pw, *ppw; long pwb[256]; + if (buf) *buf = 0; getpwuid_r(geteuid(), &pw, (void *)pwb, sizeof pwb, &ppw); if (!ppw) - return 0; + return buf; size_t len = strnlen(pw.pw_name, L_cuserid); if (len == L_cuserid) - return 0; + return buf; if (!buf) buf = usridbuf; memcpy(buf, pw.pw_name, len+1); return buf; -- 2.21.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-02-13 19:06 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-01-29 11:20 [musl] [PATCH v2] cuserid: support invocation with a NULL pointer argument Sören Tempel 2020-01-29 11:59 ` Sören Tempel 2020-03-22 9:51 ` Sören Tempel 2020-03-22 15:05 ` Rich Felker 2021-02-13 19:06 ` Rich Felker
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/musl/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).