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.wang@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 <jinpu.wang@cloud.ionos.com>
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 <fw@deneb.enyo.de>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Benjamin Drung <benjamin.drung@ionos.com>
---
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 <errno.h>
#include <gshadow.h>
#include <stdio.h>
+#include <string.h>
+#include <assert.h>
/* Define a line parsing function using the common code
used in the nss_files module. */
@@ -30,46 +32,159 @@ struct sgent_data {};
#include <nss/nss_files/files-parse.c>
+/* 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)
+ __set_errno (EINVAL);
+ return errno;
+ }
+ }
+ else if (buf[len - 1] != '\xff')
+ /* The buffer is too small. Arrange for re-reading the same
+ line on the next call. */
+ return __nss_readline_seek (fp, *poffset);
+
+ /* fgets_unlocked succeeded. */
+
+ /* Remove leading whitespace. */
+ char *p = buf;
while (isspace (*p))
- ++p;
- } 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,
- &errno));
+ ++p;
+ if (*p == '\0' || *p == '#')
+ /* Skip empty lines and comments. */
+ continue;
+ if (p != buf)
+ memmove (buf, p, strlen (p));
- _IO_funlockfile (stream);
+ /* Return line to the caller. */
+ return 0;
+ }
+}
+
+
+static int
+__nss_parse_line_result (FILE *fp, off64_t offset, int parse_line_result)
+{
+ assert (parse_line_result >= -1 && parse_line_result <= 1);
- *result = resbuf;
- return 0;
+ switch (__builtin_expect (parse_line_result, 1))
+ {
+ case 1:
+ /* Sucess. */
+ return 0;
+ case 0:
+ /* Parse error. */
+ __set_errno (EINVAL);
+ return EINVAL;
+ case -1:
+ /* Out of buffer space. */
+ return __nss_readline_seek (fp, offset);
+
+ default:
+ __builtin_unreachable ();
+ }
+}
+typedef int nss_files_parse_line (char *line, struct STRUCTURE *result,
+ struct parser_data *data,
+ size_t datalen, int *errnop);
+static int
+__nss_fgetent_r (FILE *fp, void *result, char *buffer, size_t buffer_length,
+ nss_files_parse_line parser)
+{
+ int ret;
+
+ _IO_flockfile (fp);
+
+ while (true)
+ {
+ off64_t original_offset;
+ ret = __nss_readline (fp, buffer, buffer_length, &original_offset);
+ if (ret == 0)
+ {
+ /* Parse the line into *RESULT. */
+ ret = parser (buffer, result,
+ (struct parser_data *) buffer, buffer_length, &errno);
+
+ /* Translate the result code from the parser into an errno
+ value. Also seeks back to the start of the line if
+ necessary. */
+ ret = __nss_parse_line_result (fp, original_offset, ret);
+
+ if (ret == EINVAL)
+ /* Skip over malformed lines. */
+ continue;
+ }
+ break;
+ }
+
+ _IO_funlockfile (fp);
+
+ return ret;
+}
+
+/* Read one shadow entry from the given stream. */
+int
+__fgetsgent_r (FILE *stream, struct sgrp *resbuf, char *buffer, size_t buflen,
+ struct sgrp **result)
+{
+ int ret = __nss_fgetent_r (stream, resbuf, buffer, buflen, parse_line);
+ if (ret == 0)
+ *result = resbuf;
+ else
+ *result = NULL;
+ return ret;
}
weak_alias (__fgetsgent_r, fgetsgent_r)
--
2.25.1
Reply to: