mailing list of musl libc
 help / color / mirror / code / Atom feed
* [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).