Bug#967938: libc6: systemd-sysusers SEGV due to glibc bug in fgetgsent
Dear maintainers,
We are still seeing the same SEGV with Bullseye, I did a forward
porting of the minimum bugfix.
Is it possible to get it upstream.
The patch is against glibc 2.31-13+deb11u2.
Thanks! Regards
Jinpu Wang
Sr. Linux Kernel Storage Programmer
Compute Platform Development Cloud
IONOS SE | Revaler Str. 30 | 10245 Berlin | Deutschland
Phone:
E-Mail: jinpu.w...@ionos.com | Web: www.ionos.de
Hauptsitz Montabaur, Amtsgericht Montabaur, HRB 24498
Vorstand: Hüseyin Dogan, Dr. Martin Endreß, Claudia Frese, Henning
Kettler, Arthur Mai, Britta Schmidt, Achim Weiß
Aufsichtsratsvorsitzender: Markus Kadelke
Member of United Internet
Diese E-Mail kann vertrauliche und/oder gesetzlich geschützte
Informationen enthalten. Wenn Sie nicht der bestimmungsgemäße Adressat
sind oder diese E-Mail irrtümlich erhalten haben, unterrichten Sie
bitte den Absender und vernichten Sie diese E-Mail. Anderen als dem
bestimmungsgemäßen Adressaten ist untersagt, diese E-Mail zu
speichern, weiterzuleiten oder ihren Inhalt auf welche Weise auch
immer zu verwenden.
This e-mail may contain confidential and/or privileged information. If
you are not the intended recipient of this e-mail, you are hereby
notified that saving, distribution or use of the content of this
e-mail in any way is prohibited. If you have received this e-mail in
error, please notify the sender and delete the e-mail.
From 2e7c36a5319198cfc28546a3d452f2246f050698 Mon Sep 17 00:00:00 2001
From: Jack Wang
Date: Tue, 4 Aug 2020 15:05:27 +0200
Subject: [PATCH 1/3] gshadow: Handle the parser's full buffer error code
The fgetgsent function isn't handling errors from parse_line. That
means it can run out of buffer space when adding pointers to group
members and exit early without setting all members of the static result
struct. The static result's members will remain pointing at buffer
locations from the previous line, which have been overwritten with
incompatible data, causing segfaults after it is returned normally.
https://sourceware.org/legacy-ml/libc-alpha/2016-06/msg01015.html
https://sourceware.org/bugzilla/show_bug.cgi?id=20338
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=967938
add local copies of the new functions, so that the
GLIBC_PRIVATE ABI remains unchanged, as suggested by
Florian Weimer
Signed-off-by: Jack Wang
Signed-off-by: Benjamin Drung
---
gshadow/fgetsgent_r.c | 183 ++
1 file changed, 149 insertions(+), 34 deletions(-)
diff --git a/gshadow/fgetsgent_r.c b/gshadow/fgetsgent_r.c
index a7a1860c76d3..0b15cd59733b 100644
--- a/gshadow/fgetsgent_r.c
+++ b/gshadow/fgetsgent_r.c
@@ -19,6 +19,8 @@
#include
#include
#include
+#include
+#include
/* Define a line parsing function using the common code
used in the nss_files module. */
@@ -30,46 +32,159 @@ struct sgent_data {};
#include
+/* Set the error indicator on FP. */
+static inline void
+fseterr_unlocked (FILE *fp)
+{
+ fp->_flags |= _IO_ERR_SEEN;
+}
-/* Read one shadow entry from the given stream. */
-int
-__fgetsgent_r (FILE *stream, struct sgrp *resbuf, char *buffer, size_t buflen,
- struct sgrp **result)
+static int
+__nss_readline_seek (FILE *fp, off64_t offset)
{
- char *p;
+ if (offset < 0 /* __ftello64 failed. */
+ || __fseeko64 (fp, offset, SEEK_SET) < 0)
+{
+ /* Without seeking support, it is not possible to
+ re-read the same line, so this is a hard failure. */
+ fseterr_unlocked (fp);
+ __set_errno (ESPIPE);
+ return ESPIPE;
+}
+ else
+{
+ __set_errno (ERANGE);
+ return ERANGE;
+}
+}
- _IO_flockfile (stream);
- do
+static int
+__nss_readline (FILE *fp, char *buf, size_t len, off64_t *poffset)
+{
+ /* We need space for at least one character, the line terminator,
+ and the NUL byte. */
+ if (len < 3)
{
- buffer[buflen - 1] = '\xff';
- p = fgets_unlocked (buffer, buflen, stream);
- if (p == NULL && feof_unlocked (stream))
- {
- _IO_funlockfile (stream);
- *result = NULL;
- __set_errno (ENOENT);
- return errno;
- }
- if (p == NULL || buffer[buflen - 1] != '\xff')
- {
- _IO_funlockfile (stream);
- *result = NULL;
- __set_errno (ERANGE);
- return errno;
- }
-
- /* Skip leading blanks. */
+ *poffset = -1;
+ __set_errno (ERANGE);
+ return ERANGE;
+}
+
+ while (true)
+{
+ /* Keep original offset for retries. */
+ *poffset = __ftello64 (fp);
+
+ buf[len - 1] = '\xff';/* Marker to recognize truncation. */
+ if (fgets_unlocked (buf, len, fp) == NULL)
+{
+ if (feof_unlocked (fp))
+{
+ __set_errno (ENOENT);
+ return ENOENT;
+}
+ else
+{
+ /* Any other error. Do not return ERANGE in this case
+ because the caller would retry. */
+ if (errno == ERANGE)
+
Bug#967938: libc6: systemd-sysusers SEGV due to glibc bug in fgetgsent
Hi Florian, hi Aurelien,
On Thu, Aug 6, 2020 at 12:52 PM Florian Weimer wrote:
>
> * Aurelien Jarno:
>
> > On 2020-08-06 06:08, Jinpu Wang wrote:
> >> Hi Florian,
> >>
> >> On Wed, Aug 5, 2020 at 6:44 PM Florian Weimer wrote:
> >> >
> >> > * Jinpu Wang:
> >> >
> >> > > Dear Maintainer:
> >> > >
> >> > > Sorry, add some missing information below:
> >> > >
> >> > > After update to Buster, the systemd-sysusers are segfaulting every
> >> > > time.
> >> > > After search around, I found following bugreport in glibc
> >> > > https://sourceware.org/legacy-ml/libc-alpha/2016-06/msg01015.html
> >> > >
> >> > > I backported to the fix to 2.28-10, it fixed the problem.
> >> > >
> >> > > glibc upstream have a different fix for it in 2.32, see
> >> > > https://sourceware.org/bugzilla/show_bug.cgi?id=20338
> >> > >
> >> > > I think it's still easier to backport the fix in msg01015.html to 2.28
> >> > > version,
> >> > > patch attached in the initial report.
> >> >
> >> > The patch from 2016 is incomplete because it does not seek back to the
> >> > original file position, so the next call of fgetsgent_r skips over the
> >> > entry that could not be fully parsed.
> >> Thanks for quick response, can you provide a minimum bugfix, which
> >> can be easily backported to old version like 2.28?
> >
> > I think we do not want to diverge from the upstream fix, even if it is a
> > bit more work to backport. We first need to fix it in bullseye/sid and
> > then we can try to get this in the next buster stable release.
>
> I can backport it to upstream release branches, all the way to version
> 2.28. Would that help?
>
> I plan to add local copies of the new functions, so that the
> GLIBC_PRIVATE ABI remains unchanged.
I did a backport of your fixes from glibc 2.32 to Buster 2.28, as
patch attached.
I tested with systemd-sysusers, it no longer SEGV.
Can you please review it?
>
> But I have other commitments, so that may not happen until
> September-ish.
>
> >> as you also make the bug 20338 as a security hole.
> >
> > It is marked as "security-", so it is *not* considered as a security
> > issue (as the content of this file is trusted).
>
> That's right.
Thanks!
Jinpu
From 2461138a872c868d14522553cab8a34994ebdc0e Mon Sep 17 00:00:00 2001
From: Jack Wang
Date: Tue, 4 Aug 2020 15:05:27 +0200
Subject: [PATCH 1/3] gshadow: Handle the parser's full buffer error code
The fgetgsent function isn't handling errors from parse_line. That
means it can run out of buffer space when adding pointers to group
members and exit early without setting all members of the static result
struct. The static result's members will remain pointing at buffer
locations from the previous line, which have been overwritten with
incompatible data, causing segfaults after it is returned normally.
https://sourceware.org/legacy-ml/libc-alpha/2016-06/msg01015.html
https://sourceware.org/bugzilla/show_bug.cgi?id=20338
add local copies of the new functions, so that the
GLIBC_PRIVATE ABI remains unchanged, as suggested by
Florian Weimer
Signed-off-by: Jack Wang
---
gshadow/fgetsgent_r.c | 183 ++
1 file changed, 149 insertions(+), 34 deletions(-)
diff --git a/gshadow/fgetsgent_r.c b/gshadow/fgetsgent_r.c
index 13a5b181cb9b..7cd4c795ff51 100644
--- a/gshadow/fgetsgent_r.c
+++ b/gshadow/fgetsgent_r.c
@@ -19,6 +19,8 @@
#include
#include
#include
+#include
+#include
/* Define a line parsing function using the common code
used in the nss_files module. */
@@ -30,46 +32,159 @@ struct sgent_data {};
#include
+/* Set the error indicator on FP. */
+static inline void
+fseterr_unlocked (FILE *fp)
+{
+ fp->_flags |= _IO_ERR_SEEN;
+}
-/* Read one shadow entry from the given stream. */
-int
-__fgetsgent_r (FILE *stream, struct sgrp *resbuf, char *buffer, size_t buflen,
- struct sgrp **result)
+static int
+__nss_readline_seek (FILE *fp, off64_t offset)
{
- char *p;
+ if (offset < 0 /* __ftello64 failed. */
+ || __fseeko64 (fp, offset, SEEK_SET) < 0)
+{
+ /* Without seeking support, it is not possible to
+ re-read the same line, so this is a hard failure. */
+ fseterr_unlocked (fp);
+ __set_errno (ESPIPE);
+ return ESPIPE;
+}
+ else
+{
+ __set_errno (ERANGE);
+ return ERANGE;
+}
+}
- _IO_flockfile (stream);
- do
+static int
+__nss_readline (FILE *fp, char *buf, size_t len, off64_t *poffset)
+{
+ /* We need space for at least
Bug#967938: libc6: systemd-sysusers SEGV due to glibc bug in fgetgsent
Hi Florian, On Wed, Aug 5, 2020 at 6:44 PM Florian Weimer wrote: > > * Jinpu Wang: > > > Dear Maintainer: > > > > Sorry, add some missing information below: > > > > After update to Buster, the systemd-sysusers are segfaulting every time. > > After search around, I found following bugreport in glibc > > https://sourceware.org/legacy-ml/libc-alpha/2016-06/msg01015.html > > > > I backported to the fix to 2.28-10, it fixed the problem. > > > > glibc upstream have a different fix for it in 2.32, see > > https://sourceware.org/bugzilla/show_bug.cgi?id=20338 > > > > I think it's still easier to backport the fix in msg01015.html to 2.28 > > version, > > patch attached in the initial report. > > The patch from 2016 is incomplete because it does not seek back to the > original file position, so the next call of fgetsgent_r skips over the > entry that could not be fully parsed. Thanks for quick response, can you provide a minimum bugfix, which can be easily backported to old version like 2.28? as you also make the bug 20338 as a security hole. Regards! Jinpu
Bug#967938: libc6: systemd-sysusers SEGV due to glibc bug in fgetgsent
Dear Maintainer: Sorry, add some missing information below: After update to Buster, the systemd-sysusers are segfaulting every time. After search around, I found following bugreport in glibc https://sourceware.org/legacy-ml/libc-alpha/2016-06/msg01015.html I backported to the fix to 2.28-10, it fixed the problem. glibc upstream have a different fix for it in 2.32, see https://sourceware.org/bugzilla/show_bug.cgi?id=20338 I think it's still easier to backport the fix in msg01015.html to 2.28 version, patch attached in the initial report. -- Jinpu Wang Linux Kernel Developer Application Support (IONOS Cloud) 1&1 IONOS SE | Greifswalder Str. 207 | 10405 Berlin | Germany Phone: E-mail: jinpu.w...@cloud.ionos.com | Web: www.ionos.de Hauptsitz Montabaur, Amtsgericht Montabaur, HRB 24498 Vorstand: Dr. Christian Böing, Hüseyin Dogan, Dr. Martin Endreß, Hans-Henning Kettler, Arthur Mai, Matthias Steinberg, Achim Weiß Aufsichtsratsvorsitzender: Markus Kadelke Member of United Internet Diese E-Mail kann vertrauliche und/oder gesetzlich geschützte Informationen enthalten. Wenn Sie nicht der bestimmungsgemäße Adressat sind oder diese E-Mail irrtümlich erhalten haben, unterrichten Sie bitte den Absender und vernichten Sie diese E-Mail. Anderen als dem bestimmungsgemäßen Adressaten ist untersagt, diese E-Mail zu speichern, weiterzuleiten oder ihren Inhalt auf welche Weise auch immer zu verwenden. This e-mail may contain confidential and/or privileged information. If you are not the intended recipient of this e-mail, you are hereby notified that saving, distribution or use of the content of this e-mail in any way is prohibited. If you have received this e-mail in error, please notify the sender and delete the e-mail.
Bug#967940: libc6: systemd-sysusers SEGV due to glibc bug in fgetgsent
Package: libc6 Version: 2.28-10 Severity: normal Tags: patch upstream Dear Maintainer: After update to Buster, the systemd-sysusers are segfaulting every time. After search around, I found following bugreport in glibc https://sourceware.org/legacy-ml/libc-alpha/2016-06/msg01015.html I backported to the fix to 2.28-10, it fixed the problem. glibc upstream have a different fix for it in 2.32, see https://sourceware.org/bugzilla/show_bug.cgi?id=20338 I think it's still easier to backport old 2.28 version -- System Information: Debian Release: 10.5 APT prefers stable-updates APT policy: (500, 'stable-updates'), (500, 'stable') Architecture: amd64 (x86_64) Kernel: Linux 5.4.56-pserver (SMP w/64 CPU cores) Kernel taint flags: TAINT_OOT_MODULE Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE=en_US.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /bin/dash Init: systemd (via /run/systemd/system) Versions of packages libc6 depends on: ii libgcc1 1:8.3.0-6 Versions of packages libc6 recommends: ii libidn2-0 2.0.5-1+deb10u1 Versions of packages libc6 suggests: ii debconf [debconf-2.0] 1.5.71 pn glibc-doc ii libc-l10n 2.28-10 ii locales2.28-10 -- debconf information: glibc/restart-services: glibc/kernel-not-supported: glibc/disable-screensaver: libraries/restart-without-asking: false glibc/restart-failed: glibc/upgrade: true glibc/kernel-too-old:
Bug#967938: libc6: systemd-sysusers SEGV due to glibc bug in fgetgsent
Subject: libc6: systemd-sysusers SEGV due to glibc bug in fgetgsent
Package: libc6
Version: 2.28-10
Severity: normal
Tags: patch upstream
Dear Maintainer,
*** Reporter, please consider answering these questions, where appropriate ***
* What led up to the situation?
* What exactly did you do (or not do) that was effective (or
ineffective)?
* What was the outcome of this action?
* What outcome did you expect instead?
*** End of the template - remove these template lines ***
-- System Information:
Debian Release: 10.5
APT prefers stable-updates
APT policy: (500, 'stable-updates'), (500, 'stable')
Architecture: amd64 (x86_64)
Kernel: Linux 5.4.56-pserver (SMP w/64 CPU cores)
Kernel taint flags: TAINT_OOT_MODULE
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8),
LANGUAGE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
Versions of packages libc6 depends on:
ii libgcc1 1:8.3.0-6
Versions of packages libc6 recommends:
ii libidn2-0 2.0.5-1+deb10u1
Versions of packages libc6 suggests:
ii debconf [debconf-2.0] 1.5.71
pn glibc-doc
ii libc-l10n 2.28-10
ii locales2.28-10
-- debconf information:
glibc/restart-services:
glibc/kernel-not-supported:
glibc/disable-screensaver:
libraries/restart-without-asking: false
glibc/restart-failed:
glibc/upgrade: true
glibc/kernel-too-old:
--
Jinpu Wang
Linux Kernel Developer
Application Support (IONOS Cloud)
1&1 IONOS SE | Greifswalder Str. 207 | 10405 Berlin | Germany
Phone:
E-mail: jinpu.w...@cloud.ionos.com | Web: www.ionos.de
Hauptsitz Montabaur, Amtsgericht Montabaur, HRB 24498
Vorstand: Dr. Christian Böing, Hüseyin Dogan, Dr. Martin Endreß,
Hans-Henning Kettler, Arthur Mai, Matthias Steinberg, Achim Weiß
Aufsichtsratsvorsitzender: Markus Kadelke
Member of United Internet
Diese E-Mail kann vertrauliche und/oder gesetzlich geschützte
Informationen enthalten. Wenn Sie nicht der bestimmungsgemäße Adressat
sind oder diese E-Mail irrtümlich erhalten haben, unterrichten Sie
bitte den Absender und vernichten Sie diese E-Mail. Anderen als dem
bestimmungsgemäßen Adressaten ist untersagt, diese E-Mail zu
speichern, weiterzuleiten oder ihren Inhalt auf welche Weise auch
immer zu verwenden.
This e-mail may contain confidential and/or privileged information. If
you are not the intended recipient of this e-mail, you are hereby
notified that saving, distribution or use of the content of this
e-mail in any way is prohibited. If you have received this e-mail in
error, please notify the sender and delete the e-mail.
From 6a6ac1d51ef9efcc60db0a9fa874fa9539392627 Mon Sep 17 00:00:00 2001
From: Jack Wang
Date: Tue, 4 Aug 2020 15:05:27 +0200
Subject: [PATCH 1/3] gshadow: Handle the parser's full buffer error code
The fgetgsent function isn't handling errors from parse_line. That
means it can run out of buffer space when adding pointers to group
members and exit early without setting all members of the static result
struct. The static result's members will remain pointing at buffer
locations from the previous line, which have been overwritten with
incompatible data, causing segfaults after it is returned normally.
https://sourceware.org/legacy-ml/libc-alpha/2016-06/msg01015.html
Signed-off-by: Jack Wang
---
gshadow/fgetsgent_r.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/gshadow/fgetsgent_r.c b/gshadow/fgetsgent_r.c
index 13a5b181cb9b..d41ae98b3595 100644
--- a/gshadow/fgetsgent_r.c
+++ b/gshadow/fgetsgent_r.c
@@ -37,6 +37,7 @@ __fgetsgent_r (FILE *stream, struct sgrp *resbuf, char *buffer, size_t buflen,
struct sgrp **result)
{
char *p;
+ int rc;
_IO_flockfile (stream);
do
@@ -64,11 +65,18 @@ __fgetsgent_r (FILE *stream, struct sgrp *resbuf, char *buffer, size_t buflen,
} while (*p == '\0' || *p == '#' || /* Ignore empty and comment lines. */
/* Parse the line. If it is invalid, loop to
get the next line of the file to parse. */
- ! parse_line (buffer, (void *) resbuf, (void *) buffer, buflen,
- ));
+ ! (rc =parse_line (buffer, (void *) resbuf, (void *) buffer, buflen,
+ )));
_IO_funlockfile (stream);
+ if (rc < 0)
+ {
+ *result = NULL;
+ __set_errno (ERANGE);
+ return errno;
+ }
+
*result = resbuf;
return 0;
}
--
2.25.1
