* [musl] [PATCH] remove strdupa @ 2022-10-22 13:57 Ismael Luceno 2022-10-23 5:46 ` Rich Felker 0 siblings, 1 reply; 8+ messages in thread From: Ismael Luceno @ 2022-10-22 13:57 UTC (permalink / raw) To: musl; +Cc: Rich Felker, Ismael Luceno There's no portable way to implement strdupa without double evaluation of it's parameter, and it's use leads to vulnerabilities, since there's no chance to check for stack overruns. Signed-off-by: Ismael Luceno <ismael@iodev.co.uk> --- include/string.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/string.h b/include/string.h index 43ad0942edd5..65fe0d503004 100644 --- a/include/string.h +++ b/include/string.h @@ -88,7 +88,6 @@ void explicit_bzero (void *, size_t); #endif #ifdef _GNU_SOURCE -#define strdupa(x) strcpy(alloca(strlen(x)+1),x) int strverscmp (const char *, const char *); char *strchrnul(const char *, int); char *strcasestr(const char *, const char *); -- 2.38.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [musl] [PATCH] remove strdupa 2022-10-22 13:57 [musl] [PATCH] remove strdupa Ismael Luceno @ 2022-10-23 5:46 ` Rich Felker 2022-10-23 14:10 ` Szabolcs Nagy 0 siblings, 1 reply; 8+ messages in thread From: Rich Felker @ 2022-10-23 5:46 UTC (permalink / raw) To: Ismael Luceno; +Cc: musl On Sat, Oct 22, 2022 at 03:57:23PM +0200, Ismael Luceno wrote: > There's no portable way to implement strdupa without double evaluation > of it's parameter, and it's use leads to vulnerabilities, since there's > no chance to check for stack overruns. > > Signed-off-by: Ismael Luceno <ismael@iodev.co.uk> > --- > include/string.h | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/include/string.h b/include/string.h > index 43ad0942edd5..65fe0d503004 100644 > --- a/include/string.h > +++ b/include/string.h > @@ -88,7 +88,6 @@ void explicit_bzero (void *, size_t); > #endif > > #ifdef _GNU_SOURCE > -#define strdupa(x) strcpy(alloca(strlen(x)+1),x) > int strverscmp (const char *, const char *); > char *strchrnul(const char *, int); > char *strcasestr(const char *, const char *); > -- > 2.38.1 Does anyone have strong opinions one way or the other on this -- especially distro folks who'd need to deal with the fallout? Rich ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [musl] [PATCH] remove strdupa 2022-10-23 5:46 ` Rich Felker @ 2022-10-23 14:10 ` Szabolcs Nagy 2022-10-23 15:13 ` Rich Felker 0 siblings, 1 reply; 8+ messages in thread From: Szabolcs Nagy @ 2022-10-23 14:10 UTC (permalink / raw) To: Rich Felker; +Cc: Ismael Luceno, musl * Rich Felker <dalias@libc.org> [2022-10-23 01:46:22 -0400]: > On Sat, Oct 22, 2022 at 03:57:23PM +0200, Ismael Luceno wrote: > > There's no portable way to implement strdupa without double evaluation > > of it's parameter, and it's use leads to vulnerabilities, since there's > > no chance to check for stack overruns. > > > > Signed-off-by: Ismael Luceno <ismael@iodev.co.uk> > > --- > > include/string.h | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/include/string.h b/include/string.h > > index 43ad0942edd5..65fe0d503004 100644 > > --- a/include/string.h > > +++ b/include/string.h > > @@ -88,7 +88,6 @@ void explicit_bzero (void *, size_t); > > #endif > > > > #ifdef _GNU_SOURCE > > -#define strdupa(x) strcpy(alloca(strlen(x)+1),x) > > int strverscmp (const char *, const char *); > > char *strchrnul(const char *, int); > > char *strcasestr(const char *, const char *); > > -- > > 2.38.1 > > Does anyone have strong opinions one way or the other on this -- > especially distro folks who'd need to deal with the fallout? debian code search finds strdupa in 126 packages, so it is widely used and not trivial to manually fix up, i'd expect distros to just readd that definition to avoid breakage. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [musl] [PATCH] remove strdupa 2022-10-23 14:10 ` Szabolcs Nagy @ 2022-10-23 15:13 ` Rich Felker 2022-10-23 17:03 ` Rich Felker 0 siblings, 1 reply; 8+ messages in thread From: Rich Felker @ 2022-10-23 15:13 UTC (permalink / raw) To: Ismael Luceno, musl On Sun, Oct 23, 2022 at 04:10:46PM +0200, Szabolcs Nagy wrote: > * Rich Felker <dalias@libc.org> [2022-10-23 01:46:22 -0400]: > > On Sat, Oct 22, 2022 at 03:57:23PM +0200, Ismael Luceno wrote: > > > There's no portable way to implement strdupa without double evaluation > > > of it's parameter, and it's use leads to vulnerabilities, since there's > > > no chance to check for stack overruns. > > > > > > Signed-off-by: Ismael Luceno <ismael@iodev.co.uk> > > > --- > > > include/string.h | 1 - > > > 1 file changed, 1 deletion(-) > > > > > > diff --git a/include/string.h b/include/string.h > > > index 43ad0942edd5..65fe0d503004 100644 > > > --- a/include/string.h > > > +++ b/include/string.h > > > @@ -88,7 +88,6 @@ void explicit_bzero (void *, size_t); > > > #endif > > > > > > #ifdef _GNU_SOURCE > > > -#define strdupa(x) strcpy(alloca(strlen(x)+1),x) > > > int strverscmp (const char *, const char *); > > > char *strchrnul(const char *, int); > > > char *strcasestr(const char *, const char *); > > > -- > > > 2.38.1 > > > > Does anyone have strong opinions one way or the other on this -- > > especially distro folks who'd need to deal with the fallout? > > debian code search finds strdupa in 126 packages, > so it is widely used and not trivial to manually fix up, > i'd expect distros to just readd that definition to avoid breakage. Distros can of course add it with an ugly -D'strdupa(x)=...' in CFLAGS too, or with a one-line patch to the affected packages to add the #define. I would kinda expect some of these (likely any using autotools or especially gnulib) already handle the case where it's not defined and define their own, so maybe it wouldn't actually be that much breakage (but of course this would also be a reduction in bug-catching). I think a reasonable (but unsatisfying) outcome of this thread might end up being "do nothing until there's action to greatly reduce the number of packages using strdupa" (or at least evaluate the situation and determine that most would not break). I would really like to avoid "improving" interfaces that are harmful and slated for removal, but I also don't want to make unnecessary new burden on distros/integrators/users. Rich ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [musl] [PATCH] remove strdupa 2022-10-23 15:13 ` Rich Felker @ 2022-10-23 17:03 ` Rich Felker 2022-10-24 7:58 ` Konstantin P. 0 siblings, 1 reply; 8+ messages in thread From: Rich Felker @ 2022-10-23 17:03 UTC (permalink / raw) To: Ismael Luceno, musl On Sun, Oct 23, 2022 at 11:13:42AM -0400, Rich Felker wrote: > On Sun, Oct 23, 2022 at 04:10:46PM +0200, Szabolcs Nagy wrote: > > * Rich Felker <dalias@libc.org> [2022-10-23 01:46:22 -0400]: > > > On Sat, Oct 22, 2022 at 03:57:23PM +0200, Ismael Luceno wrote: > > > > There's no portable way to implement strdupa without double evaluation > > > > of it's parameter, and it's use leads to vulnerabilities, since there's > > > > no chance to check for stack overruns. > > > > > > > > Signed-off-by: Ismael Luceno <ismael@iodev.co.uk> > > > > --- > > > > include/string.h | 1 - > > > > 1 file changed, 1 deletion(-) > > > > > > > > diff --git a/include/string.h b/include/string.h > > > > index 43ad0942edd5..65fe0d503004 100644 > > > > --- a/include/string.h > > > > +++ b/include/string.h > > > > @@ -88,7 +88,6 @@ void explicit_bzero (void *, size_t); > > > > #endif > > > > > > > > #ifdef _GNU_SOURCE > > > > -#define strdupa(x) strcpy(alloca(strlen(x)+1),x) > > > > int strverscmp (const char *, const char *); > > > > char *strchrnul(const char *, int); > > > > char *strcasestr(const char *, const char *); > > > > -- > > > > 2.38.1 > > > > > > Does anyone have strong opinions one way or the other on this -- > > > especially distro folks who'd need to deal with the fallout? > > > > debian code search finds strdupa in 126 packages, > > so it is widely used and not trivial to manually fix up, > > i'd expect distros to just readd that definition to avoid breakage. > > Distros can of course add it with an ugly -D'strdupa(x)=...' in CFLAGS > too, or with a one-line patch to the affected packages to add the > #define. I would kinda expect some of these (likely any using > autotools or especially gnulib) already handle the case where it's not > defined and define their own, so maybe it wouldn't actually be that > much breakage (but of course this would also be a reduction in > bug-catching). > > I think a reasonable (but unsatisfying) outcome of this thread might > end up being "do nothing until there's action to greatly reduce the > number of packages using strdupa" (or at least evaluate the situation > and determine that most would not break). I would really like to avoid > "improving" interfaces that are harmful and slated for removal, but I > also don't want to make unnecessary new burden on > distros/integrators/users. Extra data points from discussion on #alpine-devel: at least MacOS and FreeBSD lack a strdupa macro. This means pretty much anything using it is either non-portable OS-specific software or already has checks and fallback cases for when it doesn't exist. So the breakage from removing it might be very low. Rich ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [musl] [PATCH] remove strdupa 2022-10-23 17:03 ` Rich Felker @ 2022-10-24 7:58 ` Konstantin P. 2022-10-24 7:59 ` Joakim Sindholt 2022-10-24 14:22 ` Rich Felker 0 siblings, 2 replies; 8+ messages in thread From: Konstantin P. @ 2022-10-24 7:58 UTC (permalink / raw) To: musl [-- Attachment #1: Type: text/plain, Size: 2957 bytes --] systemd is using strdupa widely, and it is main reason why it should considered to stay. On Sun, Oct 23, 2022 at 8:03 PM Rich Felker <dalias@libc.org> wrote: > On Sun, Oct 23, 2022 at 11:13:42AM -0400, Rich Felker wrote: > > On Sun, Oct 23, 2022 at 04:10:46PM +0200, Szabolcs Nagy wrote: > > > * Rich Felker <dalias@libc.org> [2022-10-23 01:46:22 -0400]: > > > > On Sat, Oct 22, 2022 at 03:57:23PM +0200, Ismael Luceno wrote: > > > > > There's no portable way to implement strdupa without double > evaluation > > > > > of it's parameter, and it's use leads to vulnerabilities, since > there's > > > > > no chance to check for stack overruns. > > > > > > > > > > Signed-off-by: Ismael Luceno <ismael@iodev.co.uk> > > > > > --- > > > > > include/string.h | 1 - > > > > > 1 file changed, 1 deletion(-) > > > > > > > > > > diff --git a/include/string.h b/include/string.h > > > > > index 43ad0942edd5..65fe0d503004 100644 > > > > > --- a/include/string.h > > > > > +++ b/include/string.h > > > > > @@ -88,7 +88,6 @@ void explicit_bzero (void *, size_t); > > > > > #endif > > > > > > > > > > #ifdef _GNU_SOURCE > > > > > -#define strdupa(x) strcpy(alloca(strlen(x)+1),x) > > > > > int strverscmp (const char *, const char *); > > > > > char *strchrnul(const char *, int); > > > > > char *strcasestr(const char *, const char *); > > > > > -- > > > > > 2.38.1 > > > > > > > > Does anyone have strong opinions one way or the other on this -- > > > > especially distro folks who'd need to deal with the fallout? > > > > > > debian code search finds strdupa in 126 packages, > > > so it is widely used and not trivial to manually fix up, > > > i'd expect distros to just readd that definition to avoid breakage. > > > > Distros can of course add it with an ugly -D'strdupa(x)=...' in CFLAGS > > too, or with a one-line patch to the affected packages to add the > > #define. I would kinda expect some of these (likely any using > > autotools or especially gnulib) already handle the case where it's not > > defined and define their own, so maybe it wouldn't actually be that > > much breakage (but of course this would also be a reduction in > > bug-catching). > > > > I think a reasonable (but unsatisfying) outcome of this thread might > > end up being "do nothing until there's action to greatly reduce the > > number of packages using strdupa" (or at least evaluate the situation > > and determine that most would not break). I would really like to avoid > > "improving" interfaces that are harmful and slated for removal, but I > > also don't want to make unnecessary new burden on > > distros/integrators/users. > > Extra data points from discussion on #alpine-devel: at least MacOS and > FreeBSD lack a strdupa macro. This means pretty much anything using it > is either non-portable OS-specific software or already has checks and > fallback cases for when it doesn't exist. So the breakage from > removing it might be very low. > > Rich > [-- Attachment #2: Type: text/html, Size: 4016 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [musl] [PATCH] remove strdupa 2022-10-24 7:58 ` Konstantin P. @ 2022-10-24 7:59 ` Joakim Sindholt 2022-10-24 14:22 ` Rich Felker 1 sibling, 0 replies; 8+ messages in thread From: Joakim Sindholt @ 2022-10-24 7:59 UTC (permalink / raw) To: musl On Mon, 24 Oct 2022 10:58:22 +0300, "Konstantin P." <ria.freelander@gmail.com> wrote: > systemd is using strdupa widely, and it is main reason why it should > considered to stay. At git HEAD it only uses strdupa 5 times, all of them in src/test/test-string-util.c. The rest are their own str[n]dupa_safe macro that uses alloca directly. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [musl] [PATCH] remove strdupa 2022-10-24 7:58 ` Konstantin P. 2022-10-24 7:59 ` Joakim Sindholt @ 2022-10-24 14:22 ` Rich Felker 1 sibling, 0 replies; 8+ messages in thread From: Rich Felker @ 2022-10-24 14:22 UTC (permalink / raw) To: Konstantin P.; +Cc: musl On Mon, Oct 24, 2022 at 10:58:22AM +0300, Konstantin P. wrote: > systemd is using strdupa widely, and it is main reason why it should > considered to stay. This is not a reason for it to be considered to stay. Even aside from systemd intentionally not supporting anything but glibc and therefore always requiring patching to build, avoiding a one-line patch to a single package regardless of it's importance is not going to be a reason. The motivating factor here is the scale of work imposed on people preparing distributions or their own system integrations -- whether they'd be able to take care of the problem with a handful of easy patches, or fighting with many tens or hundreds of packages needing attention to get them building again. Rich ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-10-24 14:23 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-22 13:57 [musl] [PATCH] remove strdupa Ismael Luceno 2022-10-23 5:46 ` Rich Felker 2022-10-23 14:10 ` Szabolcs Nagy 2022-10-23 15:13 ` Rich Felker 2022-10-23 17:03 ` Rich Felker 2022-10-24 7:58 ` Konstantin P. 2022-10-24 7:59 ` Joakim Sindholt 2022-10-24 14:22 ` 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).