* [PATCH 00/11] Fix fgetsgent_r data corruption bug (20338)
@ 2020-07-17 8:29 Florian Weimer
2020-07-17 8:30 ` [PATCH 01/11] nss_files: Consolidate file opening in __nss_files_fopen Florian Weimer
` (11 more replies)
0 siblings, 12 replies; 24+ messages in thread
From: Florian Weimer @ 2020-07-17 8:29 UTC (permalink / raw)
To: libc-alpha
We have recently seen an uptick in reports of this bug.
The __libc_readline_unlocked removal and the nss_compat mmap fix for bug
26258 could potentially be dropped from this series.
Tested on i686-linux-gnu. I verified that the test case crashes with
glibc 2.31 on both i686 and x86-64, and not just results in failures
under valgrind.
Thanks,
Florian
Florian Weimer (11):
nss_files: Consolidate file opening in __nss_files_fopen
nss_compat: Do not use mmap to read database files (bug 26258)
nss_files: Consolidate line parse declarations in <nss_files.h>
nss_files: Use generic result pointer in parse_line
libio: Add fseterr_unlocked for internal use
nss: Add __nss_fgetent_r
grp: Implement fgetgrent_r using __nss_fgetent_r
gshadow: Implement fgetsgent_r using __nss_fgetent_r (bug 20338)
pwd: Implement fgetpwent_r using __nss_fgetent_r
shadow: Implement fgetspent_r using __nss_fgetent_r
libio: Remove __libc_readline_unlocked
grp/fgetgrent_r.c | 54 +------
gshadow/Makefile | 2 +-
gshadow/fgetsgent_r.c | 41 +----
gshadow/tst-fgetsgent_r.c | 192 +++++++++++++++++++++++
include/grp.h | 6 -
include/gshadow.h | 6 -
include/netdb.h | 13 --
include/netinet/ether.h | 6 -
include/nss_files.h | 84 ++++++++++
include/pwd.h | 6 -
include/rpc/netdb.h | 6 -
include/shadow.h | 6 -
include/stdio.h | 20 +--
libio/Makefile | 4 +-
libio/Versions | 1 -
libio/readline.c | 170 ---------------------
libio/tst-readline.c | 237 -----------------------------
nss/Makefile | 4 +-
nss/Versions | 1 +
nss/nss_compat/compat-grp.c | 6 +-
nss/nss_compat/compat-initgroups.c | 6 +-
nss/nss_compat/compat-pwd.c | 6 +-
nss/nss_compat/compat-spwd.c | 6 +-
nss/nss_fgetent_r.c | 55 +++++++
nss/nss_files/files-XXX.c | 82 ++++------
nss/nss_files/files-alias.c | 5 +-
nss/nss_files/files-initgroups.c | 6 +-
nss/nss_files/files-netgrp.c | 5 +-
nss/nss_files/files-parse.c | 6 +-
nss/nss_files_fopen.c | 47 ++++++
nss/nss_parse_line_result.c | 46 ++++++
nss/nss_readline.c | 99 ++++++++++++
pwd/fgetpwent_r.c | 43 +-----
shadow/fgetspent_r.c | 43 +-----
34 files changed, 609 insertions(+), 711 deletions(-)
create mode 100644 gshadow/tst-fgetsgent_r.c
create mode 100644 include/nss_files.h
delete mode 100644 libio/readline.c
delete mode 100644 libio/tst-readline.c
create mode 100644 nss/nss_fgetent_r.c
create mode 100644 nss/nss_files_fopen.c
create mode 100644 nss/nss_parse_line_result.c
create mode 100644 nss/nss_readline.c
--
2.26.2
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 01/11] nss_files: Consolidate file opening in __nss_files_fopen
2020-07-17 8:29 [PATCH 00/11] Fix fgetsgent_r data corruption bug (20338) Florian Weimer
@ 2020-07-17 8:30 ` Florian Weimer
2020-07-21 3:27 ` Carlos O'Donell
2020-07-17 8:30 ` [PATCH 02/11] nss_compat: Do not use mmap to read database files (bug 26258) Florian Weimer
` (10 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Florian Weimer @ 2020-07-17 8:30 UTC (permalink / raw)
To: libc-alpha
---
include/nss_files.h | 28 +++++++++++++++++++
nss/Makefile | 2 +-
nss/Versions | 1 +
nss/nss_files/files-XXX.c | 3 +-
nss/nss_files/files-alias.c | 5 ++--
nss/nss_files/files-initgroups.c | 6 ++--
nss/nss_files/files-netgrp.c | 5 ++--
nss/nss_files_fopen.c | 47 ++++++++++++++++++++++++++++++++
8 files changed, 86 insertions(+), 11 deletions(-)
create mode 100644 include/nss_files.h
create mode 100644 nss/nss_files_fopen.c
diff --git a/include/nss_files.h b/include/nss_files.h
new file mode 100644
index 0000000000..17144b7932
--- /dev/null
+++ b/include/nss_files.h
@@ -0,0 +1,28 @@
+/* Internal routines for nss_files.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef _NSS_FILES_H
+#define _NSS_FILES_H
+
+#include <stdio.h>
+
+/* Open PATH for reading, as a data source for nss_files. */
+FILE *__nss_files_fopen (const char *path);
+libc_hidden_proto (__nss_files_fopen)
+
+#endif /* _NSS_FILES_H */
diff --git a/nss/Makefile b/nss/Makefile
index cbb70167a9..00f4d89310 100644
--- a/nss/Makefile
+++ b/nss/Makefile
@@ -28,7 +28,7 @@ headers := nss.h
routines = nsswitch getnssent getnssent_r digits_dots \
valid_field valid_list_field rewrite_field \
$(addsuffix -lookup,$(databases)) \
- compat-lookup nss_hash
+ compat-lookup nss_hash nss_files_fopen
# These are the databases that go through nss dispatch.
# Caution: if you add a database here, you must add its real name
diff --git a/nss/Versions b/nss/Versions
index afc82a23c2..f489cb6eb0 100644
--- a/nss/Versions
+++ b/nss/Versions
@@ -18,6 +18,7 @@ libc {
__nss_passwd_lookup2; __nss_group_lookup2; __nss_hosts_lookup2;
__nss_services_lookup2; __nss_next2; __nss_lookup;
__nss_hash; __nss_database_lookup2;
+ __nss_files_fopen;
}
}
diff --git a/nss/nss_files/files-XXX.c b/nss/nss_files/files-XXX.c
index 73d2d5cb31..9cc5137953 100644
--- a/nss/nss_files/files-XXX.c
+++ b/nss/nss_files/files-XXX.c
@@ -22,6 +22,7 @@
#include <fcntl.h>
#include <libc-lock.h>
#include "nsswitch.h"
+#include <nss_files.h>
#include <kernel-features.h>
@@ -74,7 +75,7 @@ internal_setent (FILE **stream)
if (*stream == NULL)
{
- *stream = fopen (DATAFILE, "rce");
+ *stream = __nss_files_fopen (DATAFILE);
if (*stream == NULL)
status = errno == EAGAIN ? NSS_STATUS_TRYAGAIN : NSS_STATUS_UNAVAIL;
diff --git a/nss/nss_files/files-alias.c b/nss/nss_files/files-alias.c
index 6aff7b4c10..43fb2c49a5 100644
--- a/nss/nss_files/files-alias.c
+++ b/nss/nss_files/files-alias.c
@@ -29,6 +29,7 @@
#include <kernel-features.h>
#include "nsswitch.h"
+#include <nss_files.h>
NSS_DECLARE_MODULE_FUNCTIONS (files)
@@ -49,7 +50,7 @@ internal_setent (FILE **stream)
if (*stream == NULL)
{
- *stream = fopen ("/etc/aliases", "rce");
+ *stream = __nss_files_fopen ("/etc/aliases");
if (*stream == NULL)
status = errno == EAGAIN ? NSS_STATUS_TRYAGAIN : NSS_STATUS_UNAVAIL;
@@ -215,7 +216,7 @@ get_next_alias (FILE *stream, const char *match, struct aliasent *result,
first_unused = cp;
- listfile = fopen (&cp[9], "rce");
+ listfile = __nss_files_fopen (&cp[9]);
/* If the file does not exist we simply ignore
the statement. */
if (listfile != NULL
diff --git a/nss/nss_files/files-initgroups.c b/nss/nss_files/files-initgroups.c
index 577d6ddf1e..b6f505984a 100644
--- a/nss/nss_files/files-initgroups.c
+++ b/nss/nss_files/files-initgroups.c
@@ -26,6 +26,7 @@
#include <stdlib.h>
#include <scratch_buffer.h>
#include <nss.h>
+#include <nss_files.h>
NSS_DECLARE_MODULE_FUNCTIONS (files)
@@ -34,16 +35,13 @@ _nss_files_initgroups_dyn (const char *user, gid_t group, long int *start,
long int *size, gid_t **groupsp, long int limit,
int *errnop)
{
- FILE *stream = fopen ("/etc/group", "rce");
+ FILE *stream = __nss_files_fopen ("/etc/group");
if (stream == NULL)
{
*errnop = errno;
return *errnop == ENOMEM ? NSS_STATUS_TRYAGAIN : NSS_STATUS_UNAVAIL;
}
- /* No other thread using this stream. */
- __fsetlocking (stream, FSETLOCKING_BYCALLER);
-
char *line = NULL;
size_t linelen = 0;
enum nss_status status = NSS_STATUS_SUCCESS;
diff --git a/nss/nss_files/files-netgrp.c b/nss/nss_files/files-netgrp.c
index 2c580af01d..66e16b7c77 100644
--- a/nss/nss_files/files-netgrp.c
+++ b/nss/nss_files/files-netgrp.c
@@ -26,6 +26,7 @@
#include <string.h>
#include "nsswitch.h"
#include "netgroup.h"
+#include <nss_files.h>
NSS_DECLARE_MODULE_FUNCTIONS (files)
@@ -64,7 +65,7 @@ _nss_files_setnetgrent (const char *group, struct __netgrent *result)
return NSS_STATUS_UNAVAIL;
/* Find the netgroups file and open it. */
- fp = fopen (DATAFILE, "rce");
+ fp = __nss_files_fopen (DATAFILE);
if (fp == NULL)
status = errno == EAGAIN ? NSS_STATUS_TRYAGAIN : NSS_STATUS_UNAVAIL;
else
@@ -78,8 +79,6 @@ _nss_files_setnetgrent (const char *group, struct __netgrent *result)
status = NSS_STATUS_NOTFOUND;
result->cursor = result->data;
- __fsetlocking (fp, FSETLOCKING_BYCALLER);
-
while (!feof_unlocked (fp))
{
ssize_t curlen = getline (&line, &line_len, fp);
diff --git a/nss/nss_files_fopen.c b/nss/nss_files_fopen.c
new file mode 100644
index 0000000000..594e421657
--- /dev/null
+++ b/nss/nss_files_fopen.c
@@ -0,0 +1,47 @@
+/* Open an nss_files database file.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <nss_files.h>
+
+#include <errno.h>
+#include <stdio_ext.h>
+
+FILE *
+__nss_files_fopen (const char *path)
+{
+ FILE *fp = fopen (path, "rce");
+ if (fp == NULL)
+ return NULL;
+
+ /* The stream is not shared across threads. */
+ __fsetlocking (fp, FSETLOCKING_BYCALLER);
+
+ /* This tells libio that the file is seekable, and that fp->_offset
+ is correct, ensuring that __ftello64 is efficient (bug 26257). */
+ if (__fseeko64 (fp, 0, SEEK_SET) < 0)
+ {
+ /* nss_files requires seekable files, to deal with repeated
+ reads of the same line after reporting ERANGE. */
+ fclose (fp);
+ __set_errno (ESPIPE);
+ return NULL;
+ }
+
+ return fp;
+}
+libc_hidden_def (__nss_files_fopen)
--
2.26.2
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 02/11] nss_compat: Do not use mmap to read database files (bug 26258)
2020-07-17 8:29 [PATCH 00/11] Fix fgetsgent_r data corruption bug (20338) Florian Weimer
2020-07-17 8:30 ` [PATCH 01/11] nss_files: Consolidate file opening in __nss_files_fopen Florian Weimer
@ 2020-07-17 8:30 ` Florian Weimer
2020-07-21 3:27 ` Carlos O'Donell
2020-07-17 8:30 ` [PATCH 03/11] nss_files: Consolidate line parse declarations in <nss_files.h> Florian Weimer
` (9 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Florian Weimer @ 2020-07-17 8:30 UTC (permalink / raw)
To: libc-alpha
This avoids crashes in case the files are truncated for some reason.
For typically file sizes, it is also going to be slightly faster.
Using __nss_files_fopen instead mirrors what nss_files does.
---
nss/nss_compat/compat-grp.c | 6 ++----
nss/nss_compat/compat-initgroups.c | 6 ++----
nss/nss_compat/compat-pwd.c | 6 ++----
nss/nss_compat/compat-spwd.c | 6 ++----
4 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/nss/nss_compat/compat-grp.c b/nss/nss_compat/compat-grp.c
index d4f750b95c..510d49e8c7 100644
--- a/nss/nss_compat/compat-grp.c
+++ b/nss/nss_compat/compat-grp.c
@@ -26,6 +26,7 @@
#include <string.h>
#include <libc-lock.h>
#include <kernel-features.h>
+#include <nss_files.h>
NSS_DECLARE_MODULE_FUNCTIONS (compat)
@@ -108,13 +109,10 @@ internal_setgrent (ent_t *ent, int stayopen, int needent)
if (ent->stream == NULL)
{
- ent->stream = fopen ("/etc/group", "rme");
+ ent->stream = __nss_files_fopen ("/etc/group");
if (ent->stream == NULL)
status = errno == EAGAIN ? NSS_STATUS_TRYAGAIN : NSS_STATUS_UNAVAIL;
- else
- /* We take care of locking ourself. */
- __fsetlocking (ent->stream, FSETLOCKING_BYCALLER);
}
else
rewind (ent->stream);
diff --git a/nss/nss_compat/compat-initgroups.c b/nss/nss_compat/compat-initgroups.c
index 3671bef48b..c0dcdf839d 100644
--- a/nss/nss_compat/compat-initgroups.c
+++ b/nss/nss_compat/compat-initgroups.c
@@ -29,6 +29,7 @@
#include <libc-lock.h>
#include <kernel-features.h>
#include <scratch_buffer.h>
+#include <nss_files.h>
NSS_DECLARE_MODULE_FUNCTIONS (compat)
@@ -122,13 +123,10 @@ internal_setgrent (ent_t *ent)
else
ent->blacklist.current = 0;
- ent->stream = fopen ("/etc/group", "rme");
+ ent->stream = __nss_files_fopen ("/etc/group");
if (ent->stream == NULL)
status = errno == EAGAIN ? NSS_STATUS_TRYAGAIN : NSS_STATUS_UNAVAIL;
- else
- /* We take care of locking ourself. */
- __fsetlocking (ent->stream, FSETLOCKING_BYCALLER);
return status;
}
diff --git a/nss/nss_compat/compat-pwd.c b/nss/nss_compat/compat-pwd.c
index 394e39b811..3a212a0dab 100644
--- a/nss/nss_compat/compat-pwd.c
+++ b/nss/nss_compat/compat-pwd.c
@@ -27,6 +27,7 @@
#include <string.h>
#include <libc-lock.h>
#include <kernel-features.h>
+#include <nss_files.h>
#include "netgroup.h"
#include "nisdomain.h"
@@ -223,13 +224,10 @@ internal_setpwent (ent_t *ent, int stayopen, int needent)
if (ent->stream == NULL)
{
- ent->stream = fopen ("/etc/passwd", "rme");
+ ent->stream = __nss_files_fopen ("/etc/passwd");
if (ent->stream == NULL)
status = errno == EAGAIN ? NSS_STATUS_TRYAGAIN : NSS_STATUS_UNAVAIL;
- else
- /* We take care of locking ourself. */
- __fsetlocking (ent->stream, FSETLOCKING_BYCALLER);
}
else
rewind (ent->stream);
diff --git a/nss/nss_compat/compat-spwd.c b/nss/nss_compat/compat-spwd.c
index ec5bf283cd..d802ee0302 100644
--- a/nss/nss_compat/compat-spwd.c
+++ b/nss/nss_compat/compat-spwd.c
@@ -27,6 +27,7 @@
#include <string.h>
#include <libc-lock.h>
#include <kernel-features.h>
+#include <nss_files.h>
#include "netgroup.h"
#include "nisdomain.h"
@@ -179,13 +180,10 @@ internal_setspent (ent_t *ent, int stayopen, int needent)
if (ent->stream == NULL)
{
- ent->stream = fopen ("/etc/shadow", "rme");
+ ent->stream = __nss_files_fopen ("/etc/shadow");
if (ent->stream == NULL)
status = errno == EAGAIN ? NSS_STATUS_TRYAGAIN : NSS_STATUS_UNAVAIL;
- else
- /* We take care of locking ourself. */
- __fsetlocking (ent->stream, FSETLOCKING_BYCALLER);
}
else
rewind (ent->stream);
--
2.26.2
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 03/11] nss_files: Consolidate line parse declarations in <nss_files.h>
2020-07-17 8:29 [PATCH 00/11] Fix fgetsgent_r data corruption bug (20338) Florian Weimer
2020-07-17 8:30 ` [PATCH 01/11] nss_files: Consolidate file opening in __nss_files_fopen Florian Weimer
2020-07-17 8:30 ` [PATCH 02/11] nss_compat: Do not use mmap to read database files (bug 26258) Florian Weimer
@ 2020-07-17 8:30 ` Florian Weimer
2020-07-21 3:27 ` Carlos O'Donell
2020-07-17 8:30 ` [PATCH 04/11] nss_files: Use generic result pointer in parse_line Florian Weimer
` (8 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Florian Weimer @ 2020-07-17 8:30 UTC (permalink / raw)
To: libc-alpha
These functions should eventually have the same type, so it makes
sense to declare them together.
---
include/grp.h | 6 -----
include/gshadow.h | 6 -----
include/netdb.h | 13 ----------
include/netinet/ether.h | 6 -----
include/nss_files.h | 51 +++++++++++++++++++++++++++++++++++++
include/pwd.h | 6 -----
include/rpc/netdb.h | 6 -----
include/shadow.h | 6 -----
nss/nss_files/files-parse.c | 1 +
9 files changed, 52 insertions(+), 49 deletions(-)
diff --git a/include/grp.h b/include/grp.h
index 58f7b4d233..2cd2475534 100644
--- a/include/grp.h
+++ b/include/grp.h
@@ -30,12 +30,6 @@ extern int __old_getgrnam_r (const char *__name, struct group *__resultbuf,
char *__buffer, size_t __buflen,
struct group **__result);
-struct parser_data;
-extern int _nss_files_parse_grent (char *line, struct group *result,
- struct parser_data *data,
- size_t datalen, int *errnop);
-libc_hidden_proto (_nss_files_parse_grent)
-
#define DECLARE_NSS_PROTOTYPES(service) \
extern enum nss_status _nss_ ## service ## _setgrent (int); \
extern enum nss_status _nss_ ## service ## _endgrent (void); \
diff --git a/include/gshadow.h b/include/gshadow.h
index aa6a5a693e..1cefcfc641 100644
--- a/include/gshadow.h
+++ b/include/gshadow.h
@@ -10,11 +10,5 @@ extern int __sgetsgent_r (const char *string, struct sgrp *resbuf,
char *buffer, size_t buflen, struct sgrp **result)
attribute_hidden;
-struct parser_data;
-extern int _nss_files_parse_sgent (char *line, struct sgrp *result,
- struct parser_data *data,
- size_t datalen, int *errnop);
-libc_hidden_proto (_nss_files_parse_sgent)
-
# endif /* !_ISOMAC */
#endif
diff --git a/include/netdb.h b/include/netdb.h
index 6b431350df..49d63c1338 100644
--- a/include/netdb.h
+++ b/include/netdb.h
@@ -202,23 +202,10 @@ libc_hidden_proto (ruserpass)
#include <inet/netgroup.h>
-struct parser_data;
-extern int _nss_files_parse_protoent (char *line, struct protoent *result,
- struct parser_data *data,
- size_t datalen, int *errnop);
-extern int _nss_files_parse_servent (char *line, struct servent *result,
- struct parser_data *data,
- size_t datalen, int *errnop);
-extern int _nss_files_parse_netent (char *line, struct netent *result,
- struct parser_data *data,
- size_t datalen, int *errnop);
extern enum nss_status _nss_netgroup_parseline (char **cursor,
struct __netgrent *result,
char *buffer, size_t buflen,
int *errnop);
-libnss_files_hidden_proto (_nss_files_parse_protoent)
-libnss_files_hidden_proto (_nss_files_parse_servent)
-libnss_files_hidden_proto (_nss_files_parse_netent)
libnss_files_hidden_proto (_nss_netgroup_parseline)
#define DECLARE_NSS_PROTOTYPES(service) \
diff --git a/include/netinet/ether.h b/include/netinet/ether.h
index 8fd05f8193..1763a7e04c 100644
--- a/include/netinet/ether.h
+++ b/include/netinet/ether.h
@@ -15,12 +15,6 @@ struct etherent
struct ether_addr e_addr;
};
-struct parser_data;
-extern int _nss_files_parse_etherent (char *line, struct etherent *result,
- struct parser_data *data,
- size_t datalen, int *errnop);
-libnss_files_hidden_proto (_nss_files_parse_etherent)
-
#define DECLARE_NSS_PROTOTYPES(service) \
extern enum nss_status _nss_ ## service ## _setetherent (int __stayopen); \
extern enum nss_status _nss_ ## service ## _endetherent (void); \
diff --git a/include/nss_files.h b/include/nss_files.h
index 17144b7932..54b354afb3 100644
--- a/include/nss_files.h
+++ b/include/nss_files.h
@@ -25,4 +25,55 @@
FILE *__nss_files_fopen (const char *path);
libc_hidden_proto (__nss_files_fopen)
+struct parser_data;
+struct etherent;
+struct group;
+struct netent;
+struct passwd;
+struct protoent;
+struct rpcent;
+struct servent;
+struct sgrp;
+struct spwd;
+
+/* Instances of the parse_line function from
+ nss/nss_files/files-parse.c. */
+extern int _nss_files_parse_etherent (char *line, struct etherent *result,
+ struct parser_data *data,
+ size_t datalen, int *errnop);
+extern int _nss_files_parse_grent (char *line, struct group *result,
+ struct parser_data *data,
+ size_t datalen, int *errnop);
+extern int _nss_files_parse_netent (char *line, struct netent *result,
+ struct parser_data *data,
+ size_t datalen, int *errnop);
+extern int _nss_files_parse_protoent (char *line, struct protoent *result,
+ struct parser_data *data,
+ size_t datalen, int *errnop);
+extern int _nss_files_parse_pwent (char *line, struct passwd *result,
+ struct parser_data *data,
+ size_t datalen, int *errnop);
+extern int _nss_files_parse_rpcent (char *line, struct rpcent *result,
+ struct parser_data *data,
+ size_t datalen, int *errnop);
+extern int _nss_files_parse_servent (char *line, struct servent *result,
+ struct parser_data *data,
+ size_t datalen, int *errnop);
+extern int _nss_files_parse_sgent (char *line, struct sgrp *result,
+ struct parser_data *data,
+ size_t datalen, int *errnop);
+extern int _nss_files_parse_spent (char *line, struct spwd *result,
+ struct parser_data *data,
+ size_t datalen, int *errnop);
+
+libnss_files_hidden_proto (_nss_files_parse_etherent)
+libc_hidden_proto (_nss_files_parse_grent)
+libnss_files_hidden_proto (_nss_files_parse_netent)
+libnss_files_hidden_proto (_nss_files_parse_protoent)
+libc_hidden_proto (_nss_files_parse_pwent)
+libnss_files_hidden_proto (_nss_files_parse_rpcent)
+libnss_files_hidden_proto (_nss_files_parse_servent)
+libc_hidden_proto (_nss_files_parse_sgent)
+libc_hidden_proto (_nss_files_parse_spent)
+
#endif /* _NSS_FILES_H */
diff --git a/include/pwd.h b/include/pwd.h
index fd23fe9d6b..f8975d4957 100644
--- a/include/pwd.h
+++ b/include/pwd.h
@@ -26,12 +26,6 @@ extern int __fgetpwent_r (FILE * __stream, struct passwd *__resultbuf,
#include <nss.h>
-struct parser_data;
-extern int _nss_files_parse_pwent (char *line, struct passwd *result,
- struct parser_data *data,
- size_t datalen, int *errnop);
-libc_hidden_proto (_nss_files_parse_pwent)
-
#define DECLARE_NSS_PROTOTYPES(service) \
extern enum nss_status _nss_ ## service ## _setpwent (int); \
extern enum nss_status _nss_ ## service ## _endpwent (void); \
diff --git a/include/rpc/netdb.h b/include/rpc/netdb.h
index dc0d0e26b9..b9f591ca3b 100644
--- a/include/rpc/netdb.h
+++ b/include/rpc/netdb.h
@@ -24,12 +24,6 @@ extern int __getrpcent_r (struct rpcent *__result_buf, char *__buffer,
extern int __old_getrpcent_r (struct rpcent *__result_buf, char *__buffer,
size_t __buflen, struct rpcent **__result);
-struct parser_data;
-extern int _nss_files_parse_rpcent (char *line, struct rpcent *result,
- struct parser_data *data,
- size_t datalen, int *errnop);
-libnss_files_hidden_proto (_nss_files_parse_rpcent)
-
#define DECLARE_NSS_PROTOTYPES(service) \
extern enum nss_status _nss_ ## service ## _setrpcent (int); \
extern enum nss_status _nss_ ## service ## _endrpcent (void); \
diff --git a/include/shadow.h b/include/shadow.h
index 5168d8d4a3..fb1681909f 100644
--- a/include/shadow.h
+++ b/include/shadow.h
@@ -25,12 +25,6 @@ extern int __fgetspent_r (FILE *__stream, struct spwd *__result_buf,
extern int __lckpwdf (void);
extern int __ulckpwdf (void);
-struct parser_data;
-extern int _nss_files_parse_spent (char *line, struct spwd *result,
- struct parser_data *data,
- size_t datalen, int *errnop);
-libc_hidden_proto (_nss_files_parse_spent)
-
#define DECLARE_NSS_PROTOTYPES(service) \
extern enum nss_status _nss_ ## service ## _setspent (int); \
extern enum nss_status _nss_ ## service ## _endspent (void); \
diff --git a/nss/nss_files/files-parse.c b/nss/nss_files/files-parse.c
index a563d818f7..382028765b 100644
--- a/nss/nss_files/files-parse.c
+++ b/nss/nss_files/files-parse.c
@@ -21,6 +21,7 @@
#include <string.h>
#include <stdlib.h>
#include <stdint.h>
+#include <nss_files.h>
/* These symbols are defined by the including source file:
--
2.26.2
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 04/11] nss_files: Use generic result pointer in parse_line
2020-07-17 8:29 [PATCH 00/11] Fix fgetsgent_r data corruption bug (20338) Florian Weimer
` (2 preceding siblings ...)
2020-07-17 8:30 ` [PATCH 03/11] nss_files: Consolidate line parse declarations in <nss_files.h> Florian Weimer
@ 2020-07-17 8:30 ` Florian Weimer
2020-07-21 3:27 ` Carlos O'Donell
2020-07-17 8:30 ` [PATCH 05/11] libio: Add fseterr_unlocked for internal use Florian Weimer
` (7 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Florian Weimer @ 2020-07-17 8:30 UTC (permalink / raw)
To: libc-alpha
As a result, all parse_line functions have the same prototype, except
for that producing struct hostent. This change is ABI-compatible, so
it does not alter the internal GLIBC_PRIVATE ABI (otherwise we should
probably have renamed the exported functions).
A future change will use this to implement a generict fget*ent_r
function.
---
include/nss_files.h | 48 ++++++++++---------------------------
nss/nss_files/files-parse.c | 5 ++--
2 files changed, 15 insertions(+), 38 deletions(-)
diff --git a/include/nss_files.h b/include/nss_files.h
index 54b354afb3..d0f26815b5 100644
--- a/include/nss_files.h
+++ b/include/nss_files.h
@@ -26,45 +26,21 @@ FILE *__nss_files_fopen (const char *path);
libc_hidden_proto (__nss_files_fopen)
struct parser_data;
-struct etherent;
-struct group;
-struct netent;
-struct passwd;
-struct protoent;
-struct rpcent;
-struct servent;
-struct sgrp;
-struct spwd;
/* Instances of the parse_line function from
nss/nss_files/files-parse.c. */
-extern int _nss_files_parse_etherent (char *line, struct etherent *result,
- struct parser_data *data,
- size_t datalen, int *errnop);
-extern int _nss_files_parse_grent (char *line, struct group *result,
- struct parser_data *data,
- size_t datalen, int *errnop);
-extern int _nss_files_parse_netent (char *line, struct netent *result,
- struct parser_data *data,
- size_t datalen, int *errnop);
-extern int _nss_files_parse_protoent (char *line, struct protoent *result,
- struct parser_data *data,
- size_t datalen, int *errnop);
-extern int _nss_files_parse_pwent (char *line, struct passwd *result,
- struct parser_data *data,
- size_t datalen, int *errnop);
-extern int _nss_files_parse_rpcent (char *line, struct rpcent *result,
- struct parser_data *data,
- size_t datalen, int *errnop);
-extern int _nss_files_parse_servent (char *line, struct servent *result,
- struct parser_data *data,
- size_t datalen, int *errnop);
-extern int _nss_files_parse_sgent (char *line, struct sgrp *result,
- struct parser_data *data,
- size_t datalen, int *errnop);
-extern int _nss_files_parse_spent (char *line, struct spwd *result,
- struct parser_data *data,
- size_t datalen, int *errnop);
+typedef int nss_files_parse_line (char *line, void *result,
+ struct parser_data *data,
+ size_t datalen, int *errnop);
+extern nss_files_parse_line _nss_files_parse_etherent;
+extern nss_files_parse_line _nss_files_parse_grent;
+extern nss_files_parse_line _nss_files_parse_netent;
+extern nss_files_parse_line _nss_files_parse_protoent;
+extern nss_files_parse_line _nss_files_parse_pwent;
+extern nss_files_parse_line _nss_files_parse_rpcent;
+extern nss_files_parse_line _nss_files_parse_servent;
+extern nss_files_parse_line _nss_files_parse_sgent;
+extern nss_files_parse_line _nss_files_parse_spent;
libnss_files_hidden_proto (_nss_files_parse_etherent)
libc_hidden_proto (_nss_files_parse_grent)
diff --git a/nss/nss_files/files-parse.c b/nss/nss_files/files-parse.c
index 382028765b..c6cd43babe 100644
--- a/nss/nss_files/files-parse.c
+++ b/nss/nss_files/files-parse.c
@@ -87,7 +87,7 @@ struct parser_data
#ifdef EXTERN_PARSER
/* The parser is defined in a different module. */
-extern int parse_line (char *line, struct STRUCTURE *result,
+extern int parse_line (char *line, void *result,
struct parser_data *data, size_t datalen, int *errnop
EXTRA_ARGS_DECL);
@@ -99,10 +99,11 @@ extern int parse_line (char *line, struct STRUCTURE *result,
# define LINE_PARSER(EOLSET, BODY) \
parser_stclass int \
-parse_line (char *line, struct STRUCTURE *result, \
+parse_line (char *line, void *generic_result, \
struct parser_data *data, size_t datalen, int *errnop \
EXTRA_ARGS_DECL) \
{ \
+ struct STRUCTURE *result = generic_result; \
ENTDATA_DECL (data) \
BUFFER_PREPARE \
char *p = strpbrk (line, EOLSET "\n"); \
--
2.26.2
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 05/11] libio: Add fseterr_unlocked for internal use
2020-07-17 8:29 [PATCH 00/11] Fix fgetsgent_r data corruption bug (20338) Florian Weimer
` (3 preceding siblings ...)
2020-07-17 8:30 ` [PATCH 04/11] nss_files: Use generic result pointer in parse_line Florian Weimer
@ 2020-07-17 8:30 ` Florian Weimer
2020-07-21 3:27 ` Carlos O'Donell
2020-07-17 8:30 ` [PATCH 06/11] nss: Add __nss_fgetent_r Florian Weimer
` (6 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Florian Weimer @ 2020-07-17 8:30 UTC (permalink / raw)
To: libc-alpha
---
include/stdio.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/stdio.h b/include/stdio.h
index bc67d020d4..ede0f3d444 100644
--- a/include/stdio.h
+++ b/include/stdio.h
@@ -31,6 +31,13 @@
# define stdio_hidden_ldbl_proto(p,f) libc_hidden_proto (p ## f)
# endif
+/* Set the error indicator on FP. */
+static inline void
+fseterr_unlocked (FILE *fp)
+{
+ fp->_flags |= _IO_ERR_SEEN;
+}
+
extern int __fcloseall (void) attribute_hidden;
extern int __snprintf (char *__restrict __s, size_t __maxlen,
const char *__restrict __format, ...)
--
2.26.2
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 06/11] nss: Add __nss_fgetent_r
2020-07-17 8:29 [PATCH 00/11] Fix fgetsgent_r data corruption bug (20338) Florian Weimer
` (4 preceding siblings ...)
2020-07-17 8:30 ` [PATCH 05/11] libio: Add fseterr_unlocked for internal use Florian Weimer
@ 2020-07-17 8:30 ` Florian Weimer
2020-07-21 3:27 ` Carlos O'Donell
2020-07-17 8:30 ` [PATCH 07/11] grp: Implement fgetgrent_r using __nss_fgetent_r Florian Weimer
` (5 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Florian Weimer @ 2020-07-17 8:30 UTC (permalink / raw)
To: libc-alpha
And helper functions __nss_readline, __nss_readline_seek,
__nss_parse_line_result.
This consolidates common code for handling overlong lines and
parse files. Use the new functionality in internal_getent
in nss/nss_files/files-XXX.c.
---
include/nss_files.h | 29 +++++++++++
nss/Makefile | 4 +-
nss/Versions | 2 +-
nss/nss_fgetent_r.c | 55 +++++++++++++++++++++
nss/nss_files/files-XXX.c | 79 ++++++++++-------------------
nss/nss_parse_line_result.c | 46 +++++++++++++++++
nss/nss_readline.c | 99 +++++++++++++++++++++++++++++++++++++
7 files changed, 260 insertions(+), 54 deletions(-)
create mode 100644 nss/nss_fgetent_r.c
create mode 100644 nss/nss_parse_line_result.c
create mode 100644 nss/nss_readline.c
diff --git a/include/nss_files.h b/include/nss_files.h
index d0f26815b5..f45ea02dc0 100644
--- a/include/nss_files.h
+++ b/include/nss_files.h
@@ -25,6 +25,28 @@
FILE *__nss_files_fopen (const char *path);
libc_hidden_proto (__nss_files_fopen)
+/* Read a line from FP, storing it BUF. Strip leading blanks and skip
+ comments. Sets errno and returns error code on failure. Special
+ failure: ERANGE means the buffer is too small. The function writes
+ the original offset to *POFFSET (which can be negative in the case
+ of non-seekable input). */
+int __nss_readline (FILE *fp, char *buf, size_t len, off64_t *poffset);
+libc_hidden_proto (__nss_readline)
+
+/* Seek FP to OFFSET. Sets errno and returns error code on failure.
+ On success, sets errno to ERANGE and returns ERANGE (to indicate
+ re-reading of the same input line to the caller). If OFFSET is
+ negative, fail with ESPIPE without seeking. Intended to be used
+ after parsing data read by __nss_readline failed with ERANGE. */
+int __nss_readline_seek (FILE *fp, off64_t offset) attribute_hidden;
+
+/* Handles the result of a parse_line call (as defined by
+ nss/nss_files/files-parse.c). Adjusts the file offset of FP as
+ necessary. Returns 0 on success, and updates errno on failure (and
+ returns that error code). */
+int __nss_parse_line_result (FILE *fp, off64_t offset, int parse_line_result);
+libc_hidden_proto (__nss_parse_line_result)
+
struct parser_data;
/* Instances of the parse_line function from
@@ -52,4 +74,11 @@ libnss_files_hidden_proto (_nss_files_parse_servent)
libc_hidden_proto (_nss_files_parse_sgent)
libc_hidden_proto (_nss_files_parse_spent)
+/* Generic implementation of fget*ent_r. Reads lines from FP until
+ EOF or a successful parse into *RESULT using PARSER. Returns 0 on
+ success, ENOENT on EOF, ERANGE on too-small buffer. */
+int __nss_fgetent_r (FILE *fp, void *result,
+ char *buffer, size_t buffer_length,
+ nss_files_parse_line parser) attribute_hidden;
+
#endif /* _NSS_FILES_H */
diff --git a/nss/Makefile b/nss/Makefile
index 00f4d89310..20c412c3e1 100644
--- a/nss/Makefile
+++ b/nss/Makefile
@@ -28,7 +28,9 @@ headers := nss.h
routines = nsswitch getnssent getnssent_r digits_dots \
valid_field valid_list_field rewrite_field \
$(addsuffix -lookup,$(databases)) \
- compat-lookup nss_hash nss_files_fopen
+ compat-lookup nss_hash nss_files_fopen \
+ nss_readline nss_parse_line_result \
+ nss_fgetent_r
# These are the databases that go through nss dispatch.
# Caution: if you add a database here, you must add its real name
diff --git a/nss/Versions b/nss/Versions
index f489cb6eb0..71703750bf 100644
--- a/nss/Versions
+++ b/nss/Versions
@@ -18,7 +18,7 @@ libc {
__nss_passwd_lookup2; __nss_group_lookup2; __nss_hosts_lookup2;
__nss_services_lookup2; __nss_next2; __nss_lookup;
__nss_hash; __nss_database_lookup2;
- __nss_files_fopen;
+ __nss_files_fopen; __nss_readline; __nss_parse_line_result;
}
}
diff --git a/nss/nss_fgetent_r.c b/nss/nss_fgetent_r.c
new file mode 100644
index 0000000000..8f7c5b5cc7
--- /dev/null
+++ b/nss/nss_fgetent_r.c
@@ -0,0 +1,55 @@
+/* Generic implementation of fget*ent_r.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <errno.h>
+#include <nss_files.h>
+
+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;
+}
diff --git a/nss/nss_files/files-XXX.c b/nss/nss_files/files-XXX.c
index 9cc5137953..1db9e46127 100644
--- a/nss/nss_files/files-XXX.c
+++ b/nss/nss_files/files-XXX.c
@@ -135,10 +135,9 @@ internal_getent (FILE *stream, struct STRUCTURE *result,
char *buffer, size_t buflen, int *errnop H_ERRNO_PROTO
EXTRA_ARGS_DECL)
{
- char *p;
struct parser_data *data = (void *) buffer;
size_t linebuflen = buffer + buflen - data->linebuffer;
- int parse_result;
+ int saved_errno = errno; /* Do not clobber errno on success. */
if (buflen < sizeof *data + 2)
{
@@ -149,66 +148,42 @@ internal_getent (FILE *stream, struct STRUCTURE *result,
while (true)
{
- ssize_t r = __libc_readline_unlocked
- (stream, data->linebuffer, linebuflen);
- if (r < 0)
- {
- *errnop = errno;
- H_ERRNO_SET (NETDB_INTERNAL);
- if (*errnop == ERANGE)
- /* Request larger buffer. */
- return NSS_STATUS_TRYAGAIN;
- else
- /* Other read failure. */
- return NSS_STATUS_UNAVAIL;
- }
- else if (r == 0)
+ off64_t original_offset;
+ int ret = __nss_readline (stream, data->linebuffer, linebuflen,
+ &original_offset);
+ if (ret == ENOENT)
{
/* End of file. */
H_ERRNO_SET (HOST_NOT_FOUND);
+ __set_errno (saved_errno);
return NSS_STATUS_NOTFOUND;
}
-
- /* Everything OK. Now skip leading blanks. */
- p = data->linebuffer;
- while (isspace (*p))
- ++p;
-
- /* Ignore empty and comment lines. */
- if (*p == '\0' || *p == '#')
- continue;
-
- /* Parse the line. */
- *errnop = EINVAL;
- parse_result = parse_line (p, result, data, buflen, errnop EXTRA_ARGS);
-
- if (parse_result == -1)
+ else if (ret == 0)
{
- if (*errnop == ERANGE)
+ ret = __nss_parse_line_result (stream, original_offset,
+ parse_line (data->linebuffer,
+ result, data, buflen,
+ errnop EXTRA_ARGS));
+ if (ret == 0)
{
- /* Return to the original file position at the beginning
- of the line, so that the next call can read it again
- if necessary. */
- if (__fseeko64 (stream, -r, SEEK_CUR) != 0)
- {
- if (errno == ERANGE)
- *errnop = EINVAL;
- else
- *errnop = errno;
- H_ERRNO_SET (NETDB_INTERNAL);
- return NSS_STATUS_UNAVAIL;
- }
+ /* Line has been parsed successfully. */
+ __set_errno (saved_errno);
+ return NSS_STATUS_SUCCESS;
}
- H_ERRNO_SET (NETDB_INTERNAL);
- return NSS_STATUS_TRYAGAIN;
+ else if (ret == EINVAL)
+ /* If it is invalid, loop to get the next line of the file
+ to parse. */
+ continue;
}
- /* Return the data if parsed successfully. */
- if (parse_result != 0)
- return NSS_STATUS_SUCCESS;
-
- /* If it is invalid, loop to get the next line of the file to
- parse. */
+ *errnop = ret;
+ H_ERRNO_SET (NETDB_INTERNAL);
+ if (ret == ERANGE)
+ /* Request larger buffer. */
+ return NSS_STATUS_TRYAGAIN;
+ else
+ /* Other read failure. */
+ return NSS_STATUS_UNAVAIL;
}
}
diff --git a/nss/nss_parse_line_result.c b/nss/nss_parse_line_result.c
new file mode 100644
index 0000000000..cd008e3c36
--- /dev/null
+++ b/nss/nss_parse_line_result.c
@@ -0,0 +1,46 @@
+/* Implementation of __nss_parse_line_result.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <nss_files.h>
+
+#include <assert.h>
+#include <errno.h>
+
+int
+__nss_parse_line_result (FILE *fp, off64_t offset, int parse_line_result)
+{
+ assert (parse_line_result >= -1 && parse_line_result <= 1);
+
+ 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 ();
+ }
+}
+libc_hidden_def (__nss_parse_line_result)
diff --git a/nss/nss_readline.c b/nss/nss_readline.c
new file mode 100644
index 0000000000..44e0dd9319
--- /dev/null
+++ b/nss/nss_readline.c
@@ -0,0 +1,99 @@
+/* Read a line from an nss_files database file.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <nss_files.h>
+
+#include <ctype.h>
+#include <errno.h>
+#include <string.h>
+
+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)
+ {
+ *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;
+ if (*p == '\0' || *p == '#')
+ /* Skip empty lines and comments. */
+ continue;
+ if (p != buf)
+ memmove (buf, p, strlen (p));
+
+ /* Return line to the caller. */
+ return 0;
+ }
+}
+libc_hidden_def (__nss_readline)
+
+int
+__nss_readline_seek (FILE *fp, off64_t offset)
+{
+ 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;
+ }
+}
--
2.26.2
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 07/11] grp: Implement fgetgrent_r using __nss_fgetent_r
2020-07-17 8:29 [PATCH 00/11] Fix fgetsgent_r data corruption bug (20338) Florian Weimer
` (5 preceding siblings ...)
2020-07-17 8:30 ` [PATCH 06/11] nss: Add __nss_fgetent_r Florian Weimer
@ 2020-07-17 8:30 ` Florian Weimer
2020-07-21 3:28 ` Carlos O'Donell
2020-07-17 8:30 ` [PATCH 08/11] gshadow: Implement fgetsgent_r using __nss_fgetent_r (bug 20338) Florian Weimer
` (4 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Florian Weimer @ 2020-07-17 8:30 UTC (permalink / raw)
To: libc-alpha
---
grp/fgetgrent_r.c | 54 ++++++-----------------------------------------
1 file changed, 6 insertions(+), 48 deletions(-)
diff --git a/grp/fgetgrent_r.c b/grp/fgetgrent_r.c
index 03daf4f295..b598584830 100644
--- a/grp/fgetgrent_r.c
+++ b/grp/fgetgrent_r.c
@@ -20,10 +20,6 @@
#include <grp.h>
#include <stdio.h>
-#include <libio/iolibio.h>
-#define flockfile(s) _IO_flockfile (s)
-#define funlockfile(s) _IO_funlockfile (s)
-
/* Define a line parsing function using the common code
used in the nss_files module. */
@@ -59,49 +55,11 @@ int
__fgetgrent_r (FILE *stream, struct group *resbuf, char *buffer, size_t buflen,
struct group **result)
{
- char *p;
- int parse_result;
-
- flockfile (stream);
- do
- {
- buffer[buflen - 1] = '\xff';
- p = fgets_unlocked (buffer, buflen, stream);
- if (__builtin_expect (p == NULL, 0) && feof_unlocked (stream))
- {
- funlockfile (stream);
- *result = NULL;
- __set_errno (ENOENT);
- return errno;
- }
- if (__builtin_expect (p == NULL, 0) || buffer[buflen - 1] != '\xff')
- {
- funlockfile (stream);
- *result = NULL;
- __set_errno (ERANGE);
- return errno;
- }
-
- /* Skip leading blanks. */
- 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_result = parse_line (p, resbuf,
- (void *) buffer, buflen,
- &errno)));
-
- funlockfile (stream);
-
- if (__builtin_expect (parse_result, 0) == -1)
- {
- /* The parser ran out of space. */
- *result = NULL;
- return errno;
- }
-
- *result = resbuf;
- return 0;
+ int ret = __nss_fgetent_r (stream, resbuf, buffer, buflen, parse_line);
+ if (ret == 0)
+ *result = resbuf;
+ else
+ *result = NULL;
+ return ret;
}
weak_alias (__fgetgrent_r, fgetgrent_r)
--
2.26.2
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 08/11] gshadow: Implement fgetsgent_r using __nss_fgetent_r (bug 20338)
2020-07-17 8:29 [PATCH 00/11] Fix fgetsgent_r data corruption bug (20338) Florian Weimer
` (6 preceding siblings ...)
2020-07-17 8:30 ` [PATCH 07/11] grp: Implement fgetgrent_r using __nss_fgetent_r Florian Weimer
@ 2020-07-17 8:30 ` Florian Weimer
2020-07-21 3:28 ` Carlos O'Donell
2020-07-17 8:30 ` [PATCH 09/11] pwd: Implement fgetpwent_r using __nss_fgetent_r Florian Weimer
` (3 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Florian Weimer @ 2020-07-17 8:30 UTC (permalink / raw)
To: libc-alpha
---
gshadow/Makefile | 2 +-
gshadow/fgetsgent_r.c | 41 ++------
gshadow/tst-fgetsgent_r.c | 191 ++++++++++++++++++++++++++++++++++++++
3 files changed, 198 insertions(+), 36 deletions(-)
create mode 100644 gshadow/tst-fgetsgent_r.c
diff --git a/gshadow/Makefile b/gshadow/Makefile
index 089aa6125f..32faf7a534 100644
--- a/gshadow/Makefile
+++ b/gshadow/Makefile
@@ -26,7 +26,7 @@ headers = gshadow.h
routines = getsgent getsgnam sgetsgent fgetsgent putsgent \
getsgent_r getsgnam_r sgetsgent_r fgetsgent_r
-tests = tst-gshadow tst-putsgent
+tests = tst-gshadow tst-putsgent tst-fgetsgent_r
CFLAGS-getsgent_r.c += -fexceptions
CFLAGS-getsgent.c += -fexceptions
diff --git a/gshadow/fgetsgent_r.c b/gshadow/fgetsgent_r.c
index a7a1860c76..218206b4ac 100644
--- a/gshadow/fgetsgent_r.c
+++ b/gshadow/fgetsgent_r.c
@@ -36,40 +36,11 @@ int
__fgetsgent_r (FILE *stream, struct sgrp *resbuf, char *buffer, size_t buflen,
struct sgrp **result)
{
- char *p;
-
- _IO_flockfile (stream);
- do
- {
- 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. */
- 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));
-
- _IO_funlockfile (stream);
-
- *result = resbuf;
- return 0;
+ 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)
diff --git a/gshadow/tst-fgetsgent_r.c b/gshadow/tst-fgetsgent_r.c
new file mode 100644
index 0000000000..780409afa9
--- /dev/null
+++ b/gshadow/tst-fgetsgent_r.c
@@ -0,0 +1,191 @@
+/* Test for fgetsgent_r and buffer sizes.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <array_length.h>
+#include <errno.h>
+#include <gshadow.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <support/check.h>
+#include <support/support.h>
+#include <support/temp_file.h>
+#include <support/xmemstream.h>
+#include <support/xstdio.h>
+
+/* Turn a parsed struct back into a line string. The returned string
+ should be freed. */
+static char *
+format_ent (const struct sgrp *e)
+{
+ struct xmemstream stream;
+ xopen_memstream (&stream);
+ TEST_COMPARE (putsgent (e, stream.out), 0);
+ xfclose_memstream (&stream);
+ return stream.buffer;
+}
+
+/* An entry in the input file along with the expected output. */
+struct input
+{
+ const char *line; /* Line in the file. */
+ const char *expected; /* Expected output. NULL if skipped. */
+};
+
+const struct input inputs[] =
+ {
+ /* Regular entries. */
+ { "g1:x1::\n", "g1:x1::\n" },
+ { "g2:x2:a1:\n", "g2:x2:a1:\n" },
+ { "g3:x3:a2:u1\n", "g3:x3:a2:u1\n" },
+ { "g4:x4:a3,a4:u2,u3,u4\n", "g4:x4:a3,a4:u2,u3,u4\n" },
+
+ /* Comments and empty lines. */
+ { "\n", NULL },
+ { " \n", NULL },
+ { "\t\n", NULL },
+ { "#g:x::\n", NULL },
+ { " #g:x::\n", NULL },
+ { "\t#g:x::\n", NULL },
+ { " \t#g:x::\n", NULL },
+
+ /* Marker for synchronization. */
+ { "g5:x5::\n", "g5:x5::\n" },
+
+ /* Leading whitespace. */
+ { " g6:x6::\n", "g6:x6::\n" },
+ { "\tg7:x7::\n", "g7:x7::\n" },
+
+ /* This is expected to trigger buffer exhaustion during parsing
+ (bug 20338). */
+ {
+ "g8:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx:u5,u6,u7,u8,u9:\n",
+ "g8:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx:u5,u6,u7,u8,u9:\n",
+ },
+ {
+ "g9:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx::a5,a6,a7,a8,a9,a10\n",
+ "g9:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx::a5,a6,a7,a8,a9,a10\n",
+ },
+ };
+
+/* Writes the test data to a temporary file and returns its name. The
+ returned pointer should be freed. */
+static char *
+create_test_file (void)
+{
+ char *path;
+ int fd = create_temp_file ("tst-fgetsgent_r-", &path);
+ FILE *fp = fdopen (fd, "w");
+ TEST_VERIFY_EXIT (fp != NULL);
+
+ for (size_t i = 0; i < array_length (inputs); ++i)
+ fputs (inputs[i].line, fp);
+
+ xfclose (fp);
+ return path;
+}
+
+/* Read the test file with the indicated start buffer size. Return
+ true if the buffer size had to be increased during reading. */
+static bool
+run_test (const char *path, size_t buffer_size)
+{
+ bool resized = false;
+ FILE *fp = xfopen (path, "r");
+
+ /* This avoids repeated lseek system calls (bug 26257). */
+ TEST_COMPARE (fseeko64 (fp, 0, SEEK_SET), 0);
+
+ size_t i = 0;
+ while (true)
+ {
+ /* Skip over unused expected entries. */
+ while (i < array_length (inputs) && inputs[i].expected == NULL)
+ ++i;
+
+ /* Store the data on the heap, to help valgrind to detect
+ invalid accesses. */
+ struct sgrp *result_storage = xmalloc (sizeof (*result_storage));
+ char *buffer = xmalloc (buffer_size);
+ struct sgrp **result_pointer_storage
+ = xmalloc (sizeof (*result_pointer_storage));
+
+ int ret = fgetsgent_r (fp, result_storage, buffer, buffer_size,
+ result_pointer_storage);
+ if (ret == 0)
+ {
+ TEST_VERIFY (*result_pointer_storage != NULL);
+ TEST_VERIFY (i < array_length (inputs));
+ if (*result_pointer_storage != NULL
+ && i < array_length (inputs))
+ {
+ char * actual = format_ent (*result_pointer_storage);
+ TEST_COMPARE_STRING (inputs[i].expected, actual);
+ free (actual);
+ ++i;
+ }
+ else
+ break;
+ }
+ else
+ {
+ TEST_VERIFY (*result_pointer_storage == NULL);
+ TEST_COMPARE (ret, errno);
+
+ if (ret == ENOENT)
+ {
+ TEST_COMPARE (i, array_length (inputs));
+ free (result_pointer_storage);
+ free (buffer);
+ free (result_storage);
+ break;
+ }
+ else if (ret == ERANGE)
+ {
+ resized = true;
+ ++buffer_size;
+ }
+ else
+ FAIL_EXIT1 ("read failure: %m");
+ }
+
+ free (result_pointer_storage);
+ free (buffer);
+ free (result_storage);
+ }
+
+ return resized;
+}
+
+static int
+do_test (void)
+{
+ char *path = create_test_file ();
+
+ for (size_t buffer_size = 3; ; ++buffer_size)
+ {
+ bool resized = run_test (path, buffer_size);
+ if (!resized)
+ break;
+ }
+
+ free (path);
+
+ return 0;
+}
+
+#include <support/test-driver.c>
--
2.26.2
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 09/11] pwd: Implement fgetpwent_r using __nss_fgetent_r
2020-07-17 8:29 [PATCH 00/11] Fix fgetsgent_r data corruption bug (20338) Florian Weimer
` (7 preceding siblings ...)
2020-07-17 8:30 ` [PATCH 08/11] gshadow: Implement fgetsgent_r using __nss_fgetent_r (bug 20338) Florian Weimer
@ 2020-07-17 8:30 ` Florian Weimer
2020-07-21 3:28 ` Carlos O'Donell
2020-07-17 8:30 ` [PATCH 10/11] shadow: Implement fgetspent_r " Florian Weimer
` (2 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Florian Weimer @ 2020-07-17 8:30 UTC (permalink / raw)
To: libc-alpha
---
pwd/fgetpwent_r.c | 43 ++++++-------------------------------------
1 file changed, 6 insertions(+), 37 deletions(-)
diff --git a/pwd/fgetpwent_r.c b/pwd/fgetpwent_r.c
index 26b0ee52aa..0a2e4a55e2 100644
--- a/pwd/fgetpwent_r.c
+++ b/pwd/fgetpwent_r.c
@@ -20,9 +20,6 @@
#include <stdio.h>
#include <pwd.h>
-#define flockfile(s) _IO_flockfile (s)
-#define funlockfile(s) _IO_funlockfile (s)
-
/* Define a line parsing function using the common code
used in the nss_files module. */
@@ -72,39 +69,11 @@ int
__fgetpwent_r (FILE *stream, struct passwd *resbuf, char *buffer,
size_t buflen, struct passwd **result)
{
- char *p;
-
- flockfile (stream);
- do
- {
- buffer[buflen - 1] = '\xff';
- p = fgets_unlocked (buffer, buflen, stream);
- if (p == NULL && feof_unlocked (stream))
- {
- funlockfile (stream);
- *result = NULL;
- __set_errno (ENOENT);
- return errno;
- }
- if (p == NULL || buffer[buflen - 1] != '\xff')
- {
- funlockfile (stream);
- *result = NULL;
- __set_errno (ERANGE);
- return errno;
- }
-
- /* Skip leading blanks. */
- 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 (p, resbuf, (void *) buffer, buflen, &errno));
-
- funlockfile (stream);
-
- *result = resbuf;
- return 0;
+ int ret = __nss_fgetent_r (stream, resbuf, buffer, buflen, parse_line);
+ if (ret == 0)
+ *result = resbuf;
+ else
+ *result = NULL;
+ return ret;
}
weak_alias (__fgetpwent_r, fgetpwent_r)
--
2.26.2
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 10/11] shadow: Implement fgetspent_r using __nss_fgetent_r
2020-07-17 8:29 [PATCH 00/11] Fix fgetsgent_r data corruption bug (20338) Florian Weimer
` (8 preceding siblings ...)
2020-07-17 8:30 ` [PATCH 09/11] pwd: Implement fgetpwent_r using __nss_fgetent_r Florian Weimer
@ 2020-07-17 8:30 ` Florian Weimer
2020-07-21 3:28 ` Carlos O'Donell
2020-07-17 8:31 ` [PATCH 11/11] libio: Remove __libc_readline_unlocked Florian Weimer
2020-07-21 3:27 ` [PATCH 00/11] Fix fgetsgent_r data corruption bug (20338) Carlos O'Donell
11 siblings, 1 reply; 24+ messages in thread
From: Florian Weimer @ 2020-07-17 8:30 UTC (permalink / raw)
To: libc-alpha
---
gshadow/tst-fgetsgent_r.c | 1 +
shadow/fgetspent_r.c | 43 ++++++---------------------------------
2 files changed, 7 insertions(+), 37 deletions(-)
diff --git a/gshadow/tst-fgetsgent_r.c b/gshadow/tst-fgetsgent_r.c
index 780409afa9..f81e09b362 100644
--- a/gshadow/tst-fgetsgent_r.c
+++ b/gshadow/tst-fgetsgent_r.c
@@ -168,6 +168,7 @@ run_test (const char *path, size_t buffer_size)
free (result_storage);
}
+ xfclose (fp);
return resized;
}
diff --git a/shadow/fgetspent_r.c b/shadow/fgetspent_r.c
index da35846df0..d4d4e52f6a 100644
--- a/shadow/fgetspent_r.c
+++ b/shadow/fgetspent_r.c
@@ -20,9 +20,6 @@
#include <shadow.h>
#include <stdio.h>
-#define flockfile(s) _IO_flockfile (s)
-#define funlockfile(s) _IO_funlockfile (s)
-
/* Define a line parsing function using the common code
used in the nss_files module. */
@@ -39,39 +36,11 @@ int
__fgetspent_r (FILE *stream, struct spwd *resbuf, char *buffer, size_t buflen,
struct spwd **result)
{
- char *p;
-
- flockfile (stream);
- do
- {
- buffer[buflen - 1] = '\xff';
- p = fgets_unlocked (buffer, buflen, stream);
- if (p == NULL && feof_unlocked (stream))
- {
- funlockfile (stream);
- *result = NULL;
- __set_errno (ENOENT);
- return errno;
- }
- if (p == NULL || buffer[buflen - 1] != '\xff')
- {
- funlockfile (stream);
- *result = NULL;
- __set_errno (ERANGE);
- return errno;
- }
-
- /* Skip leading blanks. */
- 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, NULL, 0, &errno));
-
- funlockfile (stream);
-
- *result = resbuf;
- return 0;
+ int ret = __nss_fgetent_r (stream, resbuf, buffer, buflen, parse_line);
+ if (ret == 0)
+ *result = resbuf;
+ else
+ *result = NULL;
+ return ret;
}
weak_alias (__fgetspent_r, fgetspent_r)
--
2.26.2
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 11/11] libio: Remove __libc_readline_unlocked
2020-07-17 8:29 [PATCH 00/11] Fix fgetsgent_r data corruption bug (20338) Florian Weimer
` (9 preceding siblings ...)
2020-07-17 8:30 ` [PATCH 10/11] shadow: Implement fgetspent_r " Florian Weimer
@ 2020-07-17 8:31 ` Florian Weimer
2020-07-21 3:28 ` Carlos O'Donell
2020-07-21 3:27 ` [PATCH 00/11] Fix fgetsgent_r data corruption bug (20338) Carlos O'Donell
11 siblings, 1 reply; 24+ messages in thread
From: Florian Weimer @ 2020-07-17 8:31 UTC (permalink / raw)
To: libc-alpha
__nss_readline supersedes it. This reverts part of commit
3f5e3f5d066dcffb80af48ae2cf35a01a85a8f10 ("libio: Implement
internal function __libc_readline_unlocked"). The internal
aliases __fseeko64 and __ftello64 are preserved because
they are needed by __nss_readline as well.
---
include/stdio.h | 13 ---
libio/Makefile | 4 +-
libio/Versions | 1 -
libio/readline.c | 170 -------------------------------
libio/tst-readline.c | 237 -------------------------------------------
5 files changed, 2 insertions(+), 423 deletions(-)
delete mode 100644 libio/readline.c
delete mode 100644 libio/tst-readline.c
diff --git a/include/stdio.h b/include/stdio.h
index ede0f3d444..2e0dc80c16 100644
--- a/include/stdio.h
+++ b/include/stdio.h
@@ -179,19 +179,6 @@ int __vfxprintf (FILE *__fp, const char *__fmt, __gnuc_va_list,
unsigned int)
attribute_hidden;
-/* Read the next line from FP into BUFFER, of LENGTH bytes. LINE will
- include the line terminator and a NUL terminator. On success,
- return the length of the line, including the line terminator, but
- excluding the NUL termintor. On EOF, return zero and write a NUL
- terminator. On error, return -1 and set errno. If the total byte
- count (line and both terminators) exceeds LENGTH, return -1 and set
- errno to ERANGE (but do not mark the stream as failed).
-
- The behavior is undefined if FP is not seekable, or if the stream
- is already in an error state. */
-ssize_t __libc_readline_unlocked (FILE *fp, char *buffer, size_t length);
-libc_hidden_proto (__libc_readline_unlocked);
-
extern const char *const _sys_errlist_internal[] attribute_hidden;
extern const char *__get_errlist (int) attribute_hidden;
extern const char *__get_errname (int) attribute_hidden;
diff --git a/libio/Makefile b/libio/Makefile
index 926df1870b..4f0f777bbb 100644
--- a/libio/Makefile
+++ b/libio/Makefile
@@ -49,7 +49,7 @@ routines := \
__fbufsize __freading __fwriting __freadable __fwritable __flbf \
__fpurge __fpending __fsetlocking \
\
- libc_fatal fmemopen oldfmemopen vtables readline
+ libc_fatal fmemopen oldfmemopen vtables
tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc \
tst_wprintf2 tst-widetext test-fmemopen tst-ext tst-ext2 \
@@ -68,7 +68,7 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc \
tst-sprintf-ub tst-sprintf-chk-ub tst-bz24051 tst-bz24153 \
tst-wfile-sync
-tests-internal = tst-vtables tst-vtables-interposed tst-readline
+tests-internal = tst-vtables tst-vtables-interposed
ifeq (yes,$(build-shared))
# Add test-fopenloc only if shared library is enabled since it depends on
diff --git a/libio/Versions b/libio/Versions
index acb896afa9..6f1ab96100 100644
--- a/libio/Versions
+++ b/libio/Versions
@@ -161,6 +161,5 @@ libc {
__fseeko64;
__ftello64;
- __libc_readline_unlocked;
}
}
diff --git a/libio/readline.c b/libio/readline.c
deleted file mode 100644
index 99fecc5825..0000000000
--- a/libio/readline.c
+++ /dev/null
@@ -1,170 +0,0 @@
-/* fgets with ERANGE error reporting and size_t buffer length.
- Copyright (C) 2018-2020 Free Software Foundation, Inc.
- This file is part of the GNU C Library.
-
- The GNU C Library is free software; you can redistribute it and/or
- modify it under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation; either
- version 2.1 of the License, or (at your option) any later version.
-
- The GNU C Library is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
-
- You should have received a copy of the GNU Lesser General Public
- License along with the GNU C Library; if not, see
- <https://www.gnu.org/licenses/>. */
-
-#include <assert.h>
-#include <errno.h>
-#include <stdio.h>
-#include <string.h>
-
-#include "libioP.h"
-
-/* Return -1 and set errno to EINVAL if it is ERANGE. */
-static ssize_t
-fail_no_erange (void)
-{
- if (errno == ERANGE)
- __set_errno (EINVAL);
- return -1;
-}
-
-/* Slow path for reading the line. Called with no data in the stream
- read buffer. Write data to [BUFFER, BUFFER_END). */
-static ssize_t
-readline_slow (FILE *fp, char *buffer, char *buffer_end)
-{
- char *start = buffer;
-
- while (buffer < buffer_end)
- {
- if (__underflow (fp) == EOF)
- {
- if (_IO_ferror_unlocked (fp))
- /* If the EOF was caused by a read error, report it. */
- return fail_no_erange ();
- *buffer = '\0';
- /* Do not include the null terminator. */
- return buffer - start;
- }
-
- /* __underflow has filled the buffer. */
- char *readptr = fp->_IO_read_ptr;
- ssize_t readlen = fp->_IO_read_end - readptr;
- /* Make sure that __underflow really has acquired some data. */
- assert (readlen > 0);
- char *pnl = memchr (readptr, '\n', readlen);
- if (pnl != NULL)
- {
- /* We found the terminator. */
- size_t line_length = pnl - readptr;
- if (line_length + 2 > buffer_end - buffer)
- /* Not enough room in the caller-supplied buffer. */
- break;
- memcpy (buffer, readptr, line_length + 1);
- buffer[line_length + 1] = '\0';
- fp->_IO_read_ptr = pnl + 1;
- /* Do not include the null terminator. */
- return buffer - start + line_length + 1;
- }
-
- if (readlen >= buffer_end - buffer)
- /* Not enough room in the caller-supplied buffer. */
- break;
-
- /* Save and consume the stream buffer. */
- memcpy (buffer, readptr, readlen);
- fp->_IO_read_ptr = fp->_IO_read_end;
- buffer += readlen;
- }
-
- /* The line does not fit into the buffer. */
- __set_errno (ERANGE);
- return -1;
-}
-
-ssize_t
-__libc_readline_unlocked (FILE *fp, char *buffer, size_t buffer_length)
-{
- char *buffer_end = buffer + buffer_length;
-
- /* Orient the stream. */
- if (__builtin_expect (fp->_mode, -1) == 0)
- _IO_fwide (fp, -1);
-
- /* Fast path: The line terminator is found in the buffer. */
- char *readptr = fp->_IO_read_ptr;
- ssize_t readlen = fp->_IO_read_end - readptr;
- off64_t start_offset; /* File offset before reading anything. */
- if (readlen > 0)
- {
- char *pnl = memchr (readptr, '\n', readlen);
- if (pnl != NULL)
- {
- size_t line_length = pnl - readptr;
- /* Account for line and null terminators. */
- if (line_length + 2 > buffer_length)
- {
- __set_errno (ERANGE);
- return -1;
- }
- memcpy (buffer, readptr, line_length + 1);
- buffer[line_length + 1] = '\0';
- /* Consume the entire line. */
- fp->_IO_read_ptr = pnl + 1;
- return line_length + 1;
- }
-
- /* If the buffer does not have enough space for what is pending
- in the stream (plus a NUL terminator), the buffer is too
- small. */
- if (readlen + 1 > buffer_length)
- {
- __set_errno (ERANGE);
- return -1;
- }
-
- /* End of line not found. We need all the buffered data. Fall
- through to the slow path. */
- memcpy (buffer, readptr, readlen);
- buffer += readlen;
- /* The original length is invalid after this point. Use
- buffer_end instead. */
-#pragma GCC poison buffer_length
- /* Read the old offset before updating the read pointer. */
- start_offset = __ftello64 (fp);
- fp->_IO_read_ptr = fp->_IO_read_end;
- }
- else
- {
- readlen = 0;
- start_offset = __ftello64 (fp);
- }
-
- /* Slow path: Read more data from the underlying file. We need to
- restore the file pointer if the buffer is too small. First,
- check if the __ftello64 call above failed. */
- if (start_offset < 0)
- return fail_no_erange ();
-
- ssize_t result = readline_slow (fp, buffer, buffer_end);
- if (result < 0)
- {
- if (errno == ERANGE)
- {
- /* Restore the file pointer so that the caller may read the
- same line again. */
- if (__fseeko64 (fp, start_offset, SEEK_SET) < 0)
- return fail_no_erange ();
- __set_errno (ERANGE);
- }
- /* Do not restore the file position on other errors; it is
- likely that the __fseeko64 call would fail, too. */
- return -1;
- }
- return readlen + result;
-}
-libc_hidden_def (__libc_readline_unlocked)
diff --git a/libio/tst-readline.c b/libio/tst-readline.c
deleted file mode 100644
index 40663ed77f..0000000000
--- a/libio/tst-readline.c
+++ /dev/null
@@ -1,237 +0,0 @@
-/* Test the __libc_readline_unlocked function.
- Copyright (C) 2018-2020 Free Software Foundation, Inc.
- This file is part of the GNU C Library.
-
- The GNU C Library is free software; you can redistribute it and/or
- modify it under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation; either
- version 2.1 of the License, or (at your option) any later version.
-
- The GNU C Library is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
-
- You should have received a copy of the GNU Lesser General Public
- License along with the GNU C Library; if not, see
- <https://www.gnu.org/licenses/>. */
-
-/* Exercise __libc_readline_unlocked with various combinations of line
- lengths, stdio buffer sizes, and line read buffer sizes. */
-
-#include <errno.h>
-#include <stdbool.h>
-#include <stdio.h>
-#include <string.h>
-#include <support/check.h>
-#include <support/support.h>
-#include <support/temp_file.h>
-#include <support/test-driver.h>
-#include <support/xmemstream.h>
-#include <support/xstdio.h>
-#include <support/xunistd.h>
-
-enum
- {
- maximum_line_length = 7,
- number_of_lines = 3,
- };
-
-/* -1: Do not set buffer size. 0: unbuffered. Otherwise, use this as
- the size of the buffer. */
-static int buffer_size;
-
-/* These size of the buffer used for reading. Must be at least 2. */
-static int read_size;
-
-/* If a read files with ERANGE, increase the buffer size by this
- amount. Must be positive. */
-static int read_size_increment;
-
-/* If non-zero, do not reset the read size after an ERANGE error. */
-static int read_size_preserve;
-
-/* If non-zero, no '\n' at the end of the file. */
-static int no_newline_at_eof;
-
-/* Length of the line, or -1 if the line is not present. */
-static int line_lengths[number_of_lines];
-
-/* The name of the test file. */
-static char *test_file_path;
-
-/* The contents of the test file. */
-static char expected_contents[(maximum_line_length + 2) * number_of_lines + 1];
-static size_t expected_length;
-
-/* Returns a random byte which is not zero or the line terminator. */
-static char
-random_char (void)
-{
- static unsigned int rand_state = 1;
- while (true)
- {
- char result = rand_r (&rand_state) >> 16;
- if (result != 0 && result != '\n')
- return result;
- }
-}
-
-/* Create the test file. */
-static void
-prepare (int argc, char **argv)
-{
- int fd = create_temp_file ("tst-readline-", &test_file_path);
- TEST_VERIFY_EXIT (fd >= 0);
- xclose (fd);
-}
-
-/* Prepare the test file. Return false if the test parameters are
- incongruent and the test should be skipped. */
-static bool
-write_test_file (void)
-{
- expected_length = 0;
- char *p = expected_contents;
- for (int lineno = 0; lineno < number_of_lines; ++lineno)
- for (int i = 0; i < line_lengths[lineno]; ++i)
- *p++ = random_char ();
- expected_length = p - &expected_contents[0];
- if (no_newline_at_eof)
- {
- if (expected_length == 0)
- return false;
- --expected_length;
- --p;
- }
- if (test_verbose > 0)
- {
- printf ("info: writing test file of %zu bytes:\n", expected_length);
- for (int i = 0; i < number_of_lines; ++i)
- printf (" line %d: %d\n", i, line_lengths[i]);
- if (no_newline_at_eof)
- puts (" (no newline at EOF)");
- }
- TEST_VERIFY_EXIT (expected_length < sizeof (expected_contents));
- *p++ = '\0';
- support_write_file_string (test_file_path, expected_contents);
- return true;
-}
-
-/* Run a single test (a combination of a test file and read
- parameters). */
-static void
-run_test (void)
-{
- TEST_VERIFY_EXIT (read_size_increment > 0);
- if (test_verbose > 0)
- {
- printf ("info: running test: buffer_size=%d read_size=%d\n"
- " read_size_increment=%d read_size_preserve=%d\n",
- buffer_size, read_size, read_size_increment, read_size_preserve);
- }
-
- struct xmemstream result;
- xopen_memstream (&result);
-
- FILE *fp = xfopen (test_file_path, "rce");
- char *fp_buffer = NULL;
- if (buffer_size == 0)
- TEST_VERIFY_EXIT (setvbuf (fp, NULL, _IONBF, 0) == 0);
- if (buffer_size > 0)
- {
- fp_buffer = xmalloc (buffer_size);
- TEST_VERIFY_EXIT (setvbuf (fp, fp_buffer, _IOFBF, buffer_size) == 0);
- }
-
- char *line_buffer = xmalloc (read_size);
- size_t line_buffer_size = read_size;
-
- while (true)
- {
- ssize_t ret = __libc_readline_unlocked
- (fp, line_buffer, line_buffer_size);
- if (ret < 0)
- {
- TEST_VERIFY (ret == -1);
- if (errno != ERANGE)
- FAIL_EXIT1 ("__libc_readline_unlocked: %m");
- line_buffer_size += read_size_increment;
- free (line_buffer);
- line_buffer = xmalloc (line_buffer_size);
- /* Try reading this line again. */
- }
- else if (ret == 0)
- break;
- else
- {
- /* A line has been read. Save it. */
- TEST_VERIFY (ret == strlen (line_buffer));
- const char *pnl = strchr (line_buffer, '\n');
- /* If there is a \n, it must be at the end. */
- TEST_VERIFY (pnl == NULL || pnl == line_buffer + ret - 1);
- fputs (line_buffer, result.out);
-
- /* Restore the original read size if required. */
- if (line_buffer_size > read_size && !read_size_preserve)
- {
- line_buffer_size = read_size;
- free (line_buffer);
- line_buffer = xmalloc (line_buffer_size);
- }
- }
- }
-
- xfclose (fp);
- free (fp_buffer);
- free (line_buffer);
-
- xfclose_memstream (&result);
- TEST_VERIFY (result.length == expected_length);
- TEST_VERIFY (strcmp (result.buffer, expected_contents) == 0);
- if (test_verbose > 0)
- {
- printf ("info: expected (%zu): [[%s]]\n",
- expected_length, expected_contents);
- printf ("info: actual (%zu): [[%s]]\n", result.length, result.buffer);
- }
- free (result.buffer);
-}
-
-/* Test one test file with multiple read parameters. */
-static void
-test_one_file (void)
-{
- for (buffer_size = -1; buffer_size <= maximum_line_length + 1; ++buffer_size)
- for (read_size = 2; read_size <= maximum_line_length + 2; ++read_size)
- for (read_size_increment = 1; read_size_increment <= 4;
- ++read_size_increment)
- for (read_size_preserve = 0; read_size_preserve < 2;
- ++read_size_preserve)
- run_test ();
-}
-
-
-static int
-do_test (void)
-{
- /* Set up the test file contents. */
- for (line_lengths[0] = -1; line_lengths[0] <= maximum_line_length;
- ++line_lengths[0])
- for (line_lengths[1] = -1; line_lengths[1] <= maximum_line_length;
- ++line_lengths[1])
- for (line_lengths[2] = -1; line_lengths[2] <= maximum_line_length;
- ++line_lengths[2])
- for (no_newline_at_eof = 0; no_newline_at_eof < 2; ++no_newline_at_eof)
- {
- if (!write_test_file ())
- continue;
- test_one_file ();
- }
- free (test_file_path);
- return 0;
-}
-
-#define TIMEOUT 100
-#define PREPARE prepare
-#include <support/test-driver.c>
--
2.26.2
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 00/11] Fix fgetsgent_r data corruption bug (20338)
2020-07-17 8:29 [PATCH 00/11] Fix fgetsgent_r data corruption bug (20338) Florian Weimer
` (10 preceding siblings ...)
2020-07-17 8:31 ` [PATCH 11/11] libio: Remove __libc_readline_unlocked Florian Weimer
@ 2020-07-21 3:27 ` Carlos O'Donell
11 siblings, 0 replies; 24+ messages in thread
From: Carlos O'Donell @ 2020-07-21 3:27 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
On 7/17/20 4:29 AM, Florian Weimer via Libc-alpha wrote:
> We have recently seen an uptick in reports of this bug.
>
> The __libc_readline_unlocked removal and the nss_compat mmap fix for bug
> 26258 could potentially be dropped from this series.
>
> Tested on i686-linux-gnu. I verified that the test case crashes with
> glibc 2.31 on both i686 and x86-64, and not just results in failures
> under valgrind.
OK for 2.32.
Thanks for undertaking the refactor and the fix. The end result looks
much better than the existing code and uses a lot of the same functionality
but just organized better.
I tested on x86_64 and i686 also and I didn't see any failures.
Thanks for testing under valgrind which is also important.
This series is OK for 2.32 to fix the underlying bug 20338 which has
started to get visibility as it impacts systemd users.
> Thanks,
> Florian
>
> Florian Weimer (11):
> nss_files: Consolidate file opening in __nss_files_fopen
> nss_compat: Do not use mmap to read database files (bug 26258)
> nss_files: Consolidate line parse declarations in <nss_files.h>
> nss_files: Use generic result pointer in parse_line
> libio: Add fseterr_unlocked for internal use
> nss: Add __nss_fgetent_r
> grp: Implement fgetgrent_r using __nss_fgetent_r
> gshadow: Implement fgetsgent_r using __nss_fgetent_r (bug 20338)
> pwd: Implement fgetpwent_r using __nss_fgetent_r
> shadow: Implement fgetspent_r using __nss_fgetent_r
> libio: Remove __libc_readline_unlocked
>
> grp/fgetgrent_r.c | 54 +------
> gshadow/Makefile | 2 +-
> gshadow/fgetsgent_r.c | 41 +----
> gshadow/tst-fgetsgent_r.c | 192 +++++++++++++++++++++++
> include/grp.h | 6 -
> include/gshadow.h | 6 -
> include/netdb.h | 13 --
> include/netinet/ether.h | 6 -
> include/nss_files.h | 84 ++++++++++
> include/pwd.h | 6 -
> include/rpc/netdb.h | 6 -
> include/shadow.h | 6 -
> include/stdio.h | 20 +--
> libio/Makefile | 4 +-
> libio/Versions | 1 -
> libio/readline.c | 170 ---------------------
> libio/tst-readline.c | 237 -----------------------------
> nss/Makefile | 4 +-
> nss/Versions | 1 +
> nss/nss_compat/compat-grp.c | 6 +-
> nss/nss_compat/compat-initgroups.c | 6 +-
> nss/nss_compat/compat-pwd.c | 6 +-
> nss/nss_compat/compat-spwd.c | 6 +-
> nss/nss_fgetent_r.c | 55 +++++++
> nss/nss_files/files-XXX.c | 82 ++++------
> nss/nss_files/files-alias.c | 5 +-
> nss/nss_files/files-initgroups.c | 6 +-
> nss/nss_files/files-netgrp.c | 5 +-
> nss/nss_files/files-parse.c | 6 +-
> nss/nss_files_fopen.c | 47 ++++++
> nss/nss_parse_line_result.c | 46 ++++++
> nss/nss_readline.c | 99 ++++++++++++
> pwd/fgetpwent_r.c | 43 +-----
> shadow/fgetspent_r.c | 43 +-----
> 34 files changed, 609 insertions(+), 711 deletions(-)
> create mode 100644 gshadow/tst-fgetsgent_r.c
> create mode 100644 include/nss_files.h
> delete mode 100644 libio/readline.c
> delete mode 100644 libio/tst-readline.c
> create mode 100644 nss/nss_fgetent_r.c
> create mode 100644 nss/nss_files_fopen.c
> create mode 100644 nss/nss_parse_line_result.c
> create mode 100644 nss/nss_readline.c
>
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 01/11] nss_files: Consolidate file opening in __nss_files_fopen
2020-07-17 8:30 ` [PATCH 01/11] nss_files: Consolidate file opening in __nss_files_fopen Florian Weimer
@ 2020-07-21 3:27 ` Carlos O'Donell
0 siblings, 0 replies; 24+ messages in thread
From: Carlos O'Donell @ 2020-07-21 3:27 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
On 7/17/20 4:30 AM, Florian Weimer via Libc-alpha wrote:
OK for 2.32.
Introduces __nss_files_open.
Tested-by: Carlos O'Donell <carlos@redhat.com>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> ---
> include/nss_files.h | 28 +++++++++++++++++++
> nss/Makefile | 2 +-
> nss/Versions | 1 +
> nss/nss_files/files-XXX.c | 3 +-
> nss/nss_files/files-alias.c | 5 ++--
> nss/nss_files/files-initgroups.c | 6 ++--
> nss/nss_files/files-netgrp.c | 5 ++--
> nss/nss_files_fopen.c | 47 ++++++++++++++++++++++++++++++++
> 8 files changed, 86 insertions(+), 11 deletions(-)
> create mode 100644 include/nss_files.h
> create mode 100644 nss/nss_files_fopen.c
>
> diff --git a/include/nss_files.h b/include/nss_files.h
> new file mode 100644
> index 0000000000..17144b7932
> --- /dev/null
> +++ b/include/nss_files.h
> @@ -0,0 +1,28 @@
> +/* Internal routines for nss_files.
> + Copyright (C) 2020 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#ifndef _NSS_FILES_H
> +#define _NSS_FILES_H
> +
> +#include <stdio.h>
> +
> +/* Open PATH for reading, as a data source for nss_files. */
> +FILE *__nss_files_fopen (const char *path);
> +libc_hidden_proto (__nss_files_fopen)
> +
> +#endif /* _NSS_FILES_H */
> diff --git a/nss/Makefile b/nss/Makefile
> index cbb70167a9..00f4d89310 100644
> --- a/nss/Makefile
> +++ b/nss/Makefile
> @@ -28,7 +28,7 @@ headers := nss.h
> routines = nsswitch getnssent getnssent_r digits_dots \
> valid_field valid_list_field rewrite_field \
> $(addsuffix -lookup,$(databases)) \
> - compat-lookup nss_hash
> + compat-lookup nss_hash nss_files_fopen
>
> # These are the databases that go through nss dispatch.
> # Caution: if you add a database here, you must add its real name
> diff --git a/nss/Versions b/nss/Versions
> index afc82a23c2..f489cb6eb0 100644
> --- a/nss/Versions
> +++ b/nss/Versions
> @@ -18,6 +18,7 @@ libc {
> __nss_passwd_lookup2; __nss_group_lookup2; __nss_hosts_lookup2;
> __nss_services_lookup2; __nss_next2; __nss_lookup;
> __nss_hash; __nss_database_lookup2;
> + __nss_files_fopen;
> }
> }
>
> diff --git a/nss/nss_files/files-XXX.c b/nss/nss_files/files-XXX.c
> index 73d2d5cb31..9cc5137953 100644
> --- a/nss/nss_files/files-XXX.c
> +++ b/nss/nss_files/files-XXX.c
> @@ -22,6 +22,7 @@
> #include <fcntl.h>
> #include <libc-lock.h>
> #include "nsswitch.h"
> +#include <nss_files.h>
>
> #include <kernel-features.h>
>
> @@ -74,7 +75,7 @@ internal_setent (FILE **stream)
>
> if (*stream == NULL)
> {
> - *stream = fopen (DATAFILE, "rce");
> + *stream = __nss_files_fopen (DATAFILE);
>
> if (*stream == NULL)
> status = errno == EAGAIN ? NSS_STATUS_TRYAGAIN : NSS_STATUS_UNAVAIL;
> diff --git a/nss/nss_files/files-alias.c b/nss/nss_files/files-alias.c
> index 6aff7b4c10..43fb2c49a5 100644
> --- a/nss/nss_files/files-alias.c
> +++ b/nss/nss_files/files-alias.c
> @@ -29,6 +29,7 @@
> #include <kernel-features.h>
>
> #include "nsswitch.h"
> +#include <nss_files.h>
>
> NSS_DECLARE_MODULE_FUNCTIONS (files)
>
> @@ -49,7 +50,7 @@ internal_setent (FILE **stream)
>
> if (*stream == NULL)
> {
> - *stream = fopen ("/etc/aliases", "rce");
> + *stream = __nss_files_fopen ("/etc/aliases");
>
> if (*stream == NULL)
> status = errno == EAGAIN ? NSS_STATUS_TRYAGAIN : NSS_STATUS_UNAVAIL;
> @@ -215,7 +216,7 @@ get_next_alias (FILE *stream, const char *match, struct aliasent *result,
>
> first_unused = cp;
>
> - listfile = fopen (&cp[9], "rce");
> + listfile = __nss_files_fopen (&cp[9]);
> /* If the file does not exist we simply ignore
> the statement. */
> if (listfile != NULL
> diff --git a/nss/nss_files/files-initgroups.c b/nss/nss_files/files-initgroups.c
> index 577d6ddf1e..b6f505984a 100644
> --- a/nss/nss_files/files-initgroups.c
> +++ b/nss/nss_files/files-initgroups.c
> @@ -26,6 +26,7 @@
> #include <stdlib.h>
> #include <scratch_buffer.h>
> #include <nss.h>
> +#include <nss_files.h>
>
> NSS_DECLARE_MODULE_FUNCTIONS (files)
>
> @@ -34,16 +35,13 @@ _nss_files_initgroups_dyn (const char *user, gid_t group, long int *start,
> long int *size, gid_t **groupsp, long int limit,
> int *errnop)
> {
> - FILE *stream = fopen ("/etc/group", "rce");
> + FILE *stream = __nss_files_fopen ("/etc/group");
> if (stream == NULL)
> {
> *errnop = errno;
> return *errnop == ENOMEM ? NSS_STATUS_TRYAGAIN : NSS_STATUS_UNAVAIL;
> }
>
> - /* No other thread using this stream. */
> - __fsetlocking (stream, FSETLOCKING_BYCALLER);
> -
> char *line = NULL;
> size_t linelen = 0;
> enum nss_status status = NSS_STATUS_SUCCESS;
> diff --git a/nss/nss_files/files-netgrp.c b/nss/nss_files/files-netgrp.c
> index 2c580af01d..66e16b7c77 100644
> --- a/nss/nss_files/files-netgrp.c
> +++ b/nss/nss_files/files-netgrp.c
> @@ -26,6 +26,7 @@
> #include <string.h>
> #include "nsswitch.h"
> #include "netgroup.h"
> +#include <nss_files.h>
>
> NSS_DECLARE_MODULE_FUNCTIONS (files)
>
> @@ -64,7 +65,7 @@ _nss_files_setnetgrent (const char *group, struct __netgrent *result)
> return NSS_STATUS_UNAVAIL;
>
> /* Find the netgroups file and open it. */
> - fp = fopen (DATAFILE, "rce");
> + fp = __nss_files_fopen (DATAFILE);
> if (fp == NULL)
> status = errno == EAGAIN ? NSS_STATUS_TRYAGAIN : NSS_STATUS_UNAVAIL;
> else
> @@ -78,8 +79,6 @@ _nss_files_setnetgrent (const char *group, struct __netgrent *result)
> status = NSS_STATUS_NOTFOUND;
> result->cursor = result->data;
>
> - __fsetlocking (fp, FSETLOCKING_BYCALLER);
> -
> while (!feof_unlocked (fp))
> {
> ssize_t curlen = getline (&line, &line_len, fp);
> diff --git a/nss/nss_files_fopen.c b/nss/nss_files_fopen.c
> new file mode 100644
> index 0000000000..594e421657
> --- /dev/null
> +++ b/nss/nss_files_fopen.c
> @@ -0,0 +1,47 @@
> +/* Open an nss_files database file.
> + Copyright (C) 2020 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#include <nss_files.h>
> +
> +#include <errno.h>
> +#include <stdio_ext.h>
> +
> +FILE *
> +__nss_files_fopen (const char *path)
> +{
> + FILE *fp = fopen (path, "rce");
> + if (fp == NULL)
> + return NULL;
> +
> + /* The stream is not shared across threads. */
> + __fsetlocking (fp, FSETLOCKING_BYCALLER);
> +
> + /* This tells libio that the file is seekable, and that fp->_offset
> + is correct, ensuring that __ftello64 is efficient (bug 26257). */
> + if (__fseeko64 (fp, 0, SEEK_SET) < 0)
OK.
This activates the stream. After this libio is allowed to assume that the
underlying fd will not be changed by the user (which is the library itself).
> + {
> + /* nss_files requires seekable files, to deal with repeated
> + reads of the same line after reporting ERANGE. */
> + fclose (fp);
> + __set_errno (ESPIPE);
> + return NULL;
> + }
> +
> + return fp;
> +}
> +libc_hidden_def (__nss_files_fopen)
>
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 02/11] nss_compat: Do not use mmap to read database files (bug 26258)
2020-07-17 8:30 ` [PATCH 02/11] nss_compat: Do not use mmap to read database files (bug 26258) Florian Weimer
@ 2020-07-21 3:27 ` Carlos O'Donell
0 siblings, 0 replies; 24+ messages in thread
From: Carlos O'Donell @ 2020-07-21 3:27 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
On 7/17/20 4:30 AM, Florian Weimer via Libc-alpha wrote:
> This avoids crashes in case the files are truncated for some reason.
> For typically file sizes, it is also going to be slightly faster.
> Using __nss_files_fopen instead mirrors what nss_files does.
OK for 2.32.
Correct, using __nss_files_fopen activates the handle resulting in
use of the underlying caches.
Tested-by: Carlos O'Donell <carlos@redhat.com>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> ---
> nss/nss_compat/compat-grp.c | 6 ++----
> nss/nss_compat/compat-initgroups.c | 6 ++----
> nss/nss_compat/compat-pwd.c | 6 ++----
> nss/nss_compat/compat-spwd.c | 6 ++----
> 4 files changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/nss/nss_compat/compat-grp.c b/nss/nss_compat/compat-grp.c
> index d4f750b95c..510d49e8c7 100644
> --- a/nss/nss_compat/compat-grp.c
> +++ b/nss/nss_compat/compat-grp.c
> @@ -26,6 +26,7 @@
> #include <string.h>
> #include <libc-lock.h>
> #include <kernel-features.h>
> +#include <nss_files.h>
OK.
>
> NSS_DECLARE_MODULE_FUNCTIONS (compat)
>
> @@ -108,13 +109,10 @@ internal_setgrent (ent_t *ent, int stayopen, int needent)
>
> if (ent->stream == NULL)
> {
> - ent->stream = fopen ("/etc/group", "rme");
> + ent->stream = __nss_files_fopen ("/etc/group");
>
> if (ent->stream == NULL)
> status = errno == EAGAIN ? NSS_STATUS_TRYAGAIN : NSS_STATUS_UNAVAIL;
> - else
> - /* We take care of locking ourself. */
> - __fsetlocking (ent->stream, FSETLOCKING_BYCALLER);
> }
> else
> rewind (ent->stream);
> diff --git a/nss/nss_compat/compat-initgroups.c b/nss/nss_compat/compat-initgroups.c
> index 3671bef48b..c0dcdf839d 100644
> --- a/nss/nss_compat/compat-initgroups.c
> +++ b/nss/nss_compat/compat-initgroups.c
> @@ -29,6 +29,7 @@
> #include <libc-lock.h>
> #include <kernel-features.h>
> #include <scratch_buffer.h>
> +#include <nss_files.h>
>
> NSS_DECLARE_MODULE_FUNCTIONS (compat)
>
> @@ -122,13 +123,10 @@ internal_setgrent (ent_t *ent)
> else
> ent->blacklist.current = 0;
>
> - ent->stream = fopen ("/etc/group", "rme");
> + ent->stream = __nss_files_fopen ("/etc/group");
>
> if (ent->stream == NULL)
> status = errno == EAGAIN ? NSS_STATUS_TRYAGAIN : NSS_STATUS_UNAVAIL;
> - else
> - /* We take care of locking ourself. */
> - __fsetlocking (ent->stream, FSETLOCKING_BYCALLER);
>
> return status;
> }
> diff --git a/nss/nss_compat/compat-pwd.c b/nss/nss_compat/compat-pwd.c
> index 394e39b811..3a212a0dab 100644
> --- a/nss/nss_compat/compat-pwd.c
> +++ b/nss/nss_compat/compat-pwd.c
> @@ -27,6 +27,7 @@
> #include <string.h>
> #include <libc-lock.h>
> #include <kernel-features.h>
> +#include <nss_files.h>
>
> #include "netgroup.h"
> #include "nisdomain.h"
> @@ -223,13 +224,10 @@ internal_setpwent (ent_t *ent, int stayopen, int needent)
>
> if (ent->stream == NULL)
> {
> - ent->stream = fopen ("/etc/passwd", "rme");
> + ent->stream = __nss_files_fopen ("/etc/passwd");
>
> if (ent->stream == NULL)
> status = errno == EAGAIN ? NSS_STATUS_TRYAGAIN : NSS_STATUS_UNAVAIL;
> - else
> - /* We take care of locking ourself. */
> - __fsetlocking (ent->stream, FSETLOCKING_BYCALLER);
> }
> else
> rewind (ent->stream);
> diff --git a/nss/nss_compat/compat-spwd.c b/nss/nss_compat/compat-spwd.c
> index ec5bf283cd..d802ee0302 100644
> --- a/nss/nss_compat/compat-spwd.c
> +++ b/nss/nss_compat/compat-spwd.c
> @@ -27,6 +27,7 @@
> #include <string.h>
> #include <libc-lock.h>
> #include <kernel-features.h>
> +#include <nss_files.h>
>
> #include "netgroup.h"
> #include "nisdomain.h"
> @@ -179,13 +180,10 @@ internal_setspent (ent_t *ent, int stayopen, int needent)
>
> if (ent->stream == NULL)
> {
> - ent->stream = fopen ("/etc/shadow", "rme");
> + ent->stream = __nss_files_fopen ("/etc/shadow");
>
> if (ent->stream == NULL)
> status = errno == EAGAIN ? NSS_STATUS_TRYAGAIN : NSS_STATUS_UNAVAIL;
> - else
> - /* We take care of locking ourself. */
> - __fsetlocking (ent->stream, FSETLOCKING_BYCALLER);
> }
> else
> rewind (ent->stream);
>
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 03/11] nss_files: Consolidate line parse declarations in <nss_files.h>
2020-07-17 8:30 ` [PATCH 03/11] nss_files: Consolidate line parse declarations in <nss_files.h> Florian Weimer
@ 2020-07-21 3:27 ` Carlos O'Donell
0 siblings, 0 replies; 24+ messages in thread
From: Carlos O'Donell @ 2020-07-21 3:27 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
On 7/17/20 4:30 AM, Florian Weimer via Libc-alpha wrote:
> These functions should eventually have the same type, so it makes
> sense to declare them together.
OK for 2.32. Reorg looks good.
Tested-by: Carlos O'Donell <carlos@redhat.com>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> ---
> include/grp.h | 6 -----
> include/gshadow.h | 6 -----
> include/netdb.h | 13 ----------
> include/netinet/ether.h | 6 -----
> include/nss_files.h | 51 +++++++++++++++++++++++++++++++++++++
> include/pwd.h | 6 -----
> include/rpc/netdb.h | 6 -----
> include/shadow.h | 6 -----
> nss/nss_files/files-parse.c | 1 +
> 9 files changed, 52 insertions(+), 49 deletions(-)
>
> diff --git a/include/grp.h b/include/grp.h
> index 58f7b4d233..2cd2475534 100644
> --- a/include/grp.h
> +++ b/include/grp.h
> @@ -30,12 +30,6 @@ extern int __old_getgrnam_r (const char *__name, struct group *__resultbuf,
> char *__buffer, size_t __buflen,
> struct group **__result);
>
> -struct parser_data;
> -extern int _nss_files_parse_grent (char *line, struct group *result,
> - struct parser_data *data,
> - size_t datalen, int *errnop);
> -libc_hidden_proto (_nss_files_parse_grent)
> -
> #define DECLARE_NSS_PROTOTYPES(service) \
> extern enum nss_status _nss_ ## service ## _setgrent (int); \
> extern enum nss_status _nss_ ## service ## _endgrent (void); \
> diff --git a/include/gshadow.h b/include/gshadow.h
> index aa6a5a693e..1cefcfc641 100644
> --- a/include/gshadow.h
> +++ b/include/gshadow.h
> @@ -10,11 +10,5 @@ extern int __sgetsgent_r (const char *string, struct sgrp *resbuf,
> char *buffer, size_t buflen, struct sgrp **result)
> attribute_hidden;
>
> -struct parser_data;
> -extern int _nss_files_parse_sgent (char *line, struct sgrp *result,
> - struct parser_data *data,
> - size_t datalen, int *errnop);
> -libc_hidden_proto (_nss_files_parse_sgent)
> -
> # endif /* !_ISOMAC */
> #endif
> diff --git a/include/netdb.h b/include/netdb.h
> index 6b431350df..49d63c1338 100644
> --- a/include/netdb.h
> +++ b/include/netdb.h
> @@ -202,23 +202,10 @@ libc_hidden_proto (ruserpass)
>
> #include <inet/netgroup.h>
>
> -struct parser_data;
> -extern int _nss_files_parse_protoent (char *line, struct protoent *result,
> - struct parser_data *data,
> - size_t datalen, int *errnop);
> -extern int _nss_files_parse_servent (char *line, struct servent *result,
> - struct parser_data *data,
> - size_t datalen, int *errnop);
> -extern int _nss_files_parse_netent (char *line, struct netent *result,
> - struct parser_data *data,
> - size_t datalen, int *errnop);
> extern enum nss_status _nss_netgroup_parseline (char **cursor,
> struct __netgrent *result,
> char *buffer, size_t buflen,
> int *errnop);
> -libnss_files_hidden_proto (_nss_files_parse_protoent)
> -libnss_files_hidden_proto (_nss_files_parse_servent)
> -libnss_files_hidden_proto (_nss_files_parse_netent)
> libnss_files_hidden_proto (_nss_netgroup_parseline)
>
> #define DECLARE_NSS_PROTOTYPES(service) \
> diff --git a/include/netinet/ether.h b/include/netinet/ether.h
> index 8fd05f8193..1763a7e04c 100644
> --- a/include/netinet/ether.h
> +++ b/include/netinet/ether.h
> @@ -15,12 +15,6 @@ struct etherent
> struct ether_addr e_addr;
> };
>
> -struct parser_data;
> -extern int _nss_files_parse_etherent (char *line, struct etherent *result,
> - struct parser_data *data,
> - size_t datalen, int *errnop);
> -libnss_files_hidden_proto (_nss_files_parse_etherent)
> -
> #define DECLARE_NSS_PROTOTYPES(service) \
> extern enum nss_status _nss_ ## service ## _setetherent (int __stayopen); \
> extern enum nss_status _nss_ ## service ## _endetherent (void); \
> diff --git a/include/nss_files.h b/include/nss_files.h
> index 17144b7932..54b354afb3 100644
> --- a/include/nss_files.h
> +++ b/include/nss_files.h
> @@ -25,4 +25,55 @@
> FILE *__nss_files_fopen (const char *path);
> libc_hidden_proto (__nss_files_fopen)
>
> +struct parser_data;
> +struct etherent;
> +struct group;
> +struct netent;
> +struct passwd;
> +struct protoent;
> +struct rpcent;
> +struct servent;
> +struct sgrp;
> +struct spwd;
> +
> +/* Instances of the parse_line function from
> + nss/nss_files/files-parse.c. */
> +extern int _nss_files_parse_etherent (char *line, struct etherent *result,
> + struct parser_data *data,
> + size_t datalen, int *errnop);
> +extern int _nss_files_parse_grent (char *line, struct group *result,
> + struct parser_data *data,
> + size_t datalen, int *errnop);
> +extern int _nss_files_parse_netent (char *line, struct netent *result,
> + struct parser_data *data,
> + size_t datalen, int *errnop);
> +extern int _nss_files_parse_protoent (char *line, struct protoent *result,
> + struct parser_data *data,
> + size_t datalen, int *errnop);
> +extern int _nss_files_parse_pwent (char *line, struct passwd *result,
> + struct parser_data *data,
> + size_t datalen, int *errnop);
> +extern int _nss_files_parse_rpcent (char *line, struct rpcent *result,
> + struct parser_data *data,
> + size_t datalen, int *errnop);
> +extern int _nss_files_parse_servent (char *line, struct servent *result,
> + struct parser_data *data,
> + size_t datalen, int *errnop);
> +extern int _nss_files_parse_sgent (char *line, struct sgrp *result,
> + struct parser_data *data,
> + size_t datalen, int *errnop);
> +extern int _nss_files_parse_spent (char *line, struct spwd *result,
> + struct parser_data *data,
> + size_t datalen, int *errnop);
> +
> +libnss_files_hidden_proto (_nss_files_parse_etherent)
> +libc_hidden_proto (_nss_files_parse_grent)
> +libnss_files_hidden_proto (_nss_files_parse_netent)
> +libnss_files_hidden_proto (_nss_files_parse_protoent)
> +libc_hidden_proto (_nss_files_parse_pwent)
> +libnss_files_hidden_proto (_nss_files_parse_rpcent)
> +libnss_files_hidden_proto (_nss_files_parse_servent)
> +libc_hidden_proto (_nss_files_parse_sgent)
> +libc_hidden_proto (_nss_files_parse_spent)
> +
> #endif /* _NSS_FILES_H */
> diff --git a/include/pwd.h b/include/pwd.h
> index fd23fe9d6b..f8975d4957 100644
> --- a/include/pwd.h
> +++ b/include/pwd.h
> @@ -26,12 +26,6 @@ extern int __fgetpwent_r (FILE * __stream, struct passwd *__resultbuf,
>
> #include <nss.h>
>
> -struct parser_data;
> -extern int _nss_files_parse_pwent (char *line, struct passwd *result,
> - struct parser_data *data,
> - size_t datalen, int *errnop);
> -libc_hidden_proto (_nss_files_parse_pwent)
> -
> #define DECLARE_NSS_PROTOTYPES(service) \
> extern enum nss_status _nss_ ## service ## _setpwent (int); \
> extern enum nss_status _nss_ ## service ## _endpwent (void); \
> diff --git a/include/rpc/netdb.h b/include/rpc/netdb.h
> index dc0d0e26b9..b9f591ca3b 100644
> --- a/include/rpc/netdb.h
> +++ b/include/rpc/netdb.h
> @@ -24,12 +24,6 @@ extern int __getrpcent_r (struct rpcent *__result_buf, char *__buffer,
> extern int __old_getrpcent_r (struct rpcent *__result_buf, char *__buffer,
> size_t __buflen, struct rpcent **__result);
>
> -struct parser_data;
> -extern int _nss_files_parse_rpcent (char *line, struct rpcent *result,
> - struct parser_data *data,
> - size_t datalen, int *errnop);
> -libnss_files_hidden_proto (_nss_files_parse_rpcent)
> -
> #define DECLARE_NSS_PROTOTYPES(service) \
> extern enum nss_status _nss_ ## service ## _setrpcent (int); \
> extern enum nss_status _nss_ ## service ## _endrpcent (void); \
> diff --git a/include/shadow.h b/include/shadow.h
> index 5168d8d4a3..fb1681909f 100644
> --- a/include/shadow.h
> +++ b/include/shadow.h
> @@ -25,12 +25,6 @@ extern int __fgetspent_r (FILE *__stream, struct spwd *__result_buf,
> extern int __lckpwdf (void);
> extern int __ulckpwdf (void);
>
> -struct parser_data;
> -extern int _nss_files_parse_spent (char *line, struct spwd *result,
> - struct parser_data *data,
> - size_t datalen, int *errnop);
> -libc_hidden_proto (_nss_files_parse_spent)
> -
> #define DECLARE_NSS_PROTOTYPES(service) \
> extern enum nss_status _nss_ ## service ## _setspent (int); \
> extern enum nss_status _nss_ ## service ## _endspent (void); \
> diff --git a/nss/nss_files/files-parse.c b/nss/nss_files/files-parse.c
> index a563d818f7..382028765b 100644
> --- a/nss/nss_files/files-parse.c
> +++ b/nss/nss_files/files-parse.c
> @@ -21,6 +21,7 @@
> #include <string.h>
> #include <stdlib.h>
> #include <stdint.h>
> +#include <nss_files.h>
>
> /* These symbols are defined by the including source file:
>
>
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 04/11] nss_files: Use generic result pointer in parse_line
2020-07-17 8:30 ` [PATCH 04/11] nss_files: Use generic result pointer in parse_line Florian Weimer
@ 2020-07-21 3:27 ` Carlos O'Donell
0 siblings, 0 replies; 24+ messages in thread
From: Carlos O'Donell @ 2020-07-21 3:27 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
On 7/17/20 4:30 AM, Florian Weimer via Libc-alpha wrote:
> As a result, all parse_line functions have the same prototype, except
> for that producing struct hostent. This change is ABI-compatible, so
> it does not alter the internal GLIBC_PRIVATE ABI (otherwise we should
> probably have renamed the exported functions).
>
> A future change will use this to implement a generict fget*ent_r
> function.
OK for 2.32.
Tested-by: Carlos O'Donell <carlos@redhat.com>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> ---
> include/nss_files.h | 48 ++++++++++---------------------------
> nss/nss_files/files-parse.c | 5 ++--
> 2 files changed, 15 insertions(+), 38 deletions(-)
>
> diff --git a/include/nss_files.h b/include/nss_files.h
> index 54b354afb3..d0f26815b5 100644
> --- a/include/nss_files.h
> +++ b/include/nss_files.h
> @@ -26,45 +26,21 @@ FILE *__nss_files_fopen (const char *path);
> libc_hidden_proto (__nss_files_fopen)
>
> struct parser_data;
> -struct etherent;
> -struct group;
> -struct netent;
> -struct passwd;
> -struct protoent;
> -struct rpcent;
> -struct servent;
> -struct sgrp;
> -struct spwd;
>
> /* Instances of the parse_line function from
> nss/nss_files/files-parse.c. */
> -extern int _nss_files_parse_etherent (char *line, struct etherent *result,
> - struct parser_data *data,
> - size_t datalen, int *errnop);
> -extern int _nss_files_parse_grent (char *line, struct group *result,
> - struct parser_data *data,
> - size_t datalen, int *errnop);
> -extern int _nss_files_parse_netent (char *line, struct netent *result,
> - struct parser_data *data,
> - size_t datalen, int *errnop);
> -extern int _nss_files_parse_protoent (char *line, struct protoent *result,
> - struct parser_data *data,
> - size_t datalen, int *errnop);
> -extern int _nss_files_parse_pwent (char *line, struct passwd *result,
> - struct parser_data *data,
> - size_t datalen, int *errnop);
> -extern int _nss_files_parse_rpcent (char *line, struct rpcent *result,
> - struct parser_data *data,
> - size_t datalen, int *errnop);
> -extern int _nss_files_parse_servent (char *line, struct servent *result,
> - struct parser_data *data,
> - size_t datalen, int *errnop);
> -extern int _nss_files_parse_sgent (char *line, struct sgrp *result,
> - struct parser_data *data,
> - size_t datalen, int *errnop);
> -extern int _nss_files_parse_spent (char *line, struct spwd *result,
> - struct parser_data *data,
> - size_t datalen, int *errnop);
> +typedef int nss_files_parse_line (char *line, void *result,
> + struct parser_data *data,
> + size_t datalen, int *errnop);
> +extern nss_files_parse_line _nss_files_parse_etherent;
> +extern nss_files_parse_line _nss_files_parse_grent;
> +extern nss_files_parse_line _nss_files_parse_netent;
> +extern nss_files_parse_line _nss_files_parse_protoent;
> +extern nss_files_parse_line _nss_files_parse_pwent;
> +extern nss_files_parse_line _nss_files_parse_rpcent;
> +extern nss_files_parse_line _nss_files_parse_servent;
> +extern nss_files_parse_line _nss_files_parse_sgent;
> +extern nss_files_parse_line _nss_files_parse_spent;
>
> libnss_files_hidden_proto (_nss_files_parse_etherent)
> libc_hidden_proto (_nss_files_parse_grent)
> diff --git a/nss/nss_files/files-parse.c b/nss/nss_files/files-parse.c
> index 382028765b..c6cd43babe 100644
> --- a/nss/nss_files/files-parse.c
> +++ b/nss/nss_files/files-parse.c
> @@ -87,7 +87,7 @@ struct parser_data
> #ifdef EXTERN_PARSER
>
> /* The parser is defined in a different module. */
> -extern int parse_line (char *line, struct STRUCTURE *result,
> +extern int parse_line (char *line, void *result,
> struct parser_data *data, size_t datalen, int *errnop
> EXTRA_ARGS_DECL);
>
> @@ -99,10 +99,11 @@ extern int parse_line (char *line, struct STRUCTURE *result,
>
> # define LINE_PARSER(EOLSET, BODY) \
> parser_stclass int \
> -parse_line (char *line, struct STRUCTURE *result, \
> +parse_line (char *line, void *generic_result, \
> struct parser_data *data, size_t datalen, int *errnop \
> EXTRA_ARGS_DECL) \
> { \
> + struct STRUCTURE *result = generic_result; \
> ENTDATA_DECL (data) \
> BUFFER_PREPARE \
> char *p = strpbrk (line, EOLSET "\n"); \
>
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 05/11] libio: Add fseterr_unlocked for internal use
2020-07-17 8:30 ` [PATCH 05/11] libio: Add fseterr_unlocked for internal use Florian Weimer
@ 2020-07-21 3:27 ` Carlos O'Donell
0 siblings, 0 replies; 24+ messages in thread
From: Carlos O'Donell @ 2020-07-21 3:27 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
On 7/17/20 4:30 AM, Florian Weimer via Libc-alpha wrote:
OK for 2.32.
Tested-by: Carlos O'Donell <carlos@redhat.com>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> ---
> include/stdio.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/include/stdio.h b/include/stdio.h
> index bc67d020d4..ede0f3d444 100644
> --- a/include/stdio.h
> +++ b/include/stdio.h
> @@ -31,6 +31,13 @@
> # define stdio_hidden_ldbl_proto(p,f) libc_hidden_proto (p ## f)
> # endif
>
> +/* Set the error indicator on FP. */
> +static inline void
> +fseterr_unlocked (FILE *fp)
> +{
> + fp->_flags |= _IO_ERR_SEEN;
> +}
> +
> extern int __fcloseall (void) attribute_hidden;
> extern int __snprintf (char *__restrict __s, size_t __maxlen,
> const char *__restrict __format, ...)
>
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 06/11] nss: Add __nss_fgetent_r
2020-07-17 8:30 ` [PATCH 06/11] nss: Add __nss_fgetent_r Florian Weimer
@ 2020-07-21 3:27 ` Carlos O'Donell
0 siblings, 0 replies; 24+ messages in thread
From: Carlos O'Donell @ 2020-07-21 3:27 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
On 7/17/20 4:30 AM, Florian Weimer via Libc-alpha wrote:
> And helper functions __nss_readline, __nss_readline_seek,
> __nss_parse_line_result.
>
> This consolidates common code for handling overlong lines and
> parse files. Use the new functionality in internal_getent
> in nss/nss_files/files-XXX.c.
OK for 2.32. Very clean!
Tested-by: Carlos O'Donell <carlos@redhat.com>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> ---
> include/nss_files.h | 29 +++++++++++
> nss/Makefile | 4 +-
> nss/Versions | 2 +-
> nss/nss_fgetent_r.c | 55 +++++++++++++++++++++
> nss/nss_files/files-XXX.c | 79 ++++++++++-------------------
> nss/nss_parse_line_result.c | 46 +++++++++++++++++
> nss/nss_readline.c | 99 +++++++++++++++++++++++++++++++++++++
> 7 files changed, 260 insertions(+), 54 deletions(-)
> create mode 100644 nss/nss_fgetent_r.c
> create mode 100644 nss/nss_parse_line_result.c
> create mode 100644 nss/nss_readline.c
>
> diff --git a/include/nss_files.h b/include/nss_files.h
> index d0f26815b5..f45ea02dc0 100644
> --- a/include/nss_files.h
> +++ b/include/nss_files.h
> @@ -25,6 +25,28 @@
> FILE *__nss_files_fopen (const char *path);
> libc_hidden_proto (__nss_files_fopen)
>
> +/* Read a line from FP, storing it BUF. Strip leading blanks and skip
> + comments. Sets errno and returns error code on failure. Special
> + failure: ERANGE means the buffer is too small. The function writes
> + the original offset to *POFFSET (which can be negative in the case
> + of non-seekable input). */
> +int __nss_readline (FILE *fp, char *buf, size_t len, off64_t *poffset);
> +libc_hidden_proto (__nss_readline)
> +
> +/* Seek FP to OFFSET. Sets errno and returns error code on failure.
> + On success, sets errno to ERANGE and returns ERANGE (to indicate
> + re-reading of the same input line to the caller). If OFFSET is
> + negative, fail with ESPIPE without seeking. Intended to be used
> + after parsing data read by __nss_readline failed with ERANGE. */
> +int __nss_readline_seek (FILE *fp, off64_t offset) attribute_hidden;
> +
> +/* Handles the result of a parse_line call (as defined by
> + nss/nss_files/files-parse.c). Adjusts the file offset of FP as
> + necessary. Returns 0 on success, and updates errno on failure (and
> + returns that error code). */
> +int __nss_parse_line_result (FILE *fp, off64_t offset, int parse_line_result);
> +libc_hidden_proto (__nss_parse_line_result)
> +
> struct parser_data;
>
> /* Instances of the parse_line function from
> @@ -52,4 +74,11 @@ libnss_files_hidden_proto (_nss_files_parse_servent)
> libc_hidden_proto (_nss_files_parse_sgent)
> libc_hidden_proto (_nss_files_parse_spent)
>
> +/* Generic implementation of fget*ent_r. Reads lines from FP until
> + EOF or a successful parse into *RESULT using PARSER. Returns 0 on
> + success, ENOENT on EOF, ERANGE on too-small buffer. */
> +int __nss_fgetent_r (FILE *fp, void *result,
> + char *buffer, size_t buffer_length,
> + nss_files_parse_line parser) attribute_hidden;
> +
> #endif /* _NSS_FILES_H */
> diff --git a/nss/Makefile b/nss/Makefile
> index 00f4d89310..20c412c3e1 100644
> --- a/nss/Makefile
> +++ b/nss/Makefile
> @@ -28,7 +28,9 @@ headers := nss.h
> routines = nsswitch getnssent getnssent_r digits_dots \
> valid_field valid_list_field rewrite_field \
> $(addsuffix -lookup,$(databases)) \
> - compat-lookup nss_hash nss_files_fopen
> + compat-lookup nss_hash nss_files_fopen \
> + nss_readline nss_parse_line_result \
> + nss_fgetent_r
>
> # These are the databases that go through nss dispatch.
> # Caution: if you add a database here, you must add its real name
> diff --git a/nss/Versions b/nss/Versions
> index f489cb6eb0..71703750bf 100644
> --- a/nss/Versions
> +++ b/nss/Versions
> @@ -18,7 +18,7 @@ libc {
> __nss_passwd_lookup2; __nss_group_lookup2; __nss_hosts_lookup2;
> __nss_services_lookup2; __nss_next2; __nss_lookup;
> __nss_hash; __nss_database_lookup2;
> - __nss_files_fopen;
> + __nss_files_fopen; __nss_readline; __nss_parse_line_result;
> }
> }
>
> diff --git a/nss/nss_fgetent_r.c b/nss/nss_fgetent_r.c
> new file mode 100644
> index 0000000000..8f7c5b5cc7
> --- /dev/null
> +++ b/nss/nss_fgetent_r.c
> @@ -0,0 +1,55 @@
> +/* Generic implementation of fget*ent_r.
> + Copyright (C) 2020 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#include <errno.h>
> +#include <nss_files.h>
> +
> +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;
> +}
> diff --git a/nss/nss_files/files-XXX.c b/nss/nss_files/files-XXX.c
> index 9cc5137953..1db9e46127 100644
> --- a/nss/nss_files/files-XXX.c
> +++ b/nss/nss_files/files-XXX.c
> @@ -135,10 +135,9 @@ internal_getent (FILE *stream, struct STRUCTURE *result,
> char *buffer, size_t buflen, int *errnop H_ERRNO_PROTO
> EXTRA_ARGS_DECL)
> {
> - char *p;
> struct parser_data *data = (void *) buffer;
> size_t linebuflen = buffer + buflen - data->linebuffer;
> - int parse_result;
> + int saved_errno = errno; /* Do not clobber errno on success. */
>
> if (buflen < sizeof *data + 2)
> {
> @@ -149,66 +148,42 @@ internal_getent (FILE *stream, struct STRUCTURE *result,
>
> while (true)
> {
> - ssize_t r = __libc_readline_unlocked
> - (stream, data->linebuffer, linebuflen);
> - if (r < 0)
> - {
> - *errnop = errno;
> - H_ERRNO_SET (NETDB_INTERNAL);
> - if (*errnop == ERANGE)
> - /* Request larger buffer. */
> - return NSS_STATUS_TRYAGAIN;
> - else
> - /* Other read failure. */
> - return NSS_STATUS_UNAVAIL;
> - }
> - else if (r == 0)
> + off64_t original_offset;
> + int ret = __nss_readline (stream, data->linebuffer, linebuflen,
> + &original_offset);
OK.
> + if (ret == ENOENT)
> {
> /* End of file. */
> H_ERRNO_SET (HOST_NOT_FOUND);
> + __set_errno (saved_errno);
> return NSS_STATUS_NOTFOUND;
> }
> -
> - /* Everything OK. Now skip leading blanks. */
> - p = data->linebuffer;
> - while (isspace (*p))
> - ++p;
> -
> - /* Ignore empty and comment lines. */
> - if (*p == '\0' || *p == '#')
> - continue;
> -
> - /* Parse the line. */
> - *errnop = EINVAL;
> - parse_result = parse_line (p, result, data, buflen, errnop EXTRA_ARGS);
> -
> - if (parse_result == -1)
> + else if (ret == 0)
> {
> - if (*errnop == ERANGE)
> + ret = __nss_parse_line_result (stream, original_offset,
> + parse_line (data->linebuffer,
> + result, data, buflen,
> + errnop EXTRA_ARGS));
OK.
> + if (ret == 0)
> {
> - /* Return to the original file position at the beginning
> - of the line, so that the next call can read it again
> - if necessary. */
> - if (__fseeko64 (stream, -r, SEEK_CUR) != 0)
> - {
> - if (errno == ERANGE)
> - *errnop = EINVAL;
> - else
> - *errnop = errno;
> - H_ERRNO_SET (NETDB_INTERNAL);
> - return NSS_STATUS_UNAVAIL;
> - }
> + /* Line has been parsed successfully. */
> + __set_errno (saved_errno);
> + return NSS_STATUS_SUCCESS;
> }
> - H_ERRNO_SET (NETDB_INTERNAL);
> - return NSS_STATUS_TRYAGAIN;
> + else if (ret == EINVAL)
> + /* If it is invalid, loop to get the next line of the file
> + to parse. */
> + continue;
> }
>
> - /* Return the data if parsed successfully. */
> - if (parse_result != 0)
> - return NSS_STATUS_SUCCESS;
> -
> - /* If it is invalid, loop to get the next line of the file to
> - parse. */
> + *errnop = ret;
> + H_ERRNO_SET (NETDB_INTERNAL);
> + if (ret == ERANGE)
> + /* Request larger buffer. */
> + return NSS_STATUS_TRYAGAIN;
> + else
> + /* Other read failure. */
> + return NSS_STATUS_UNAVAIL;
> }
> }
>
> diff --git a/nss/nss_parse_line_result.c b/nss/nss_parse_line_result.c
> new file mode 100644
> index 0000000000..cd008e3c36
> --- /dev/null
> +++ b/nss/nss_parse_line_result.c
> @@ -0,0 +1,46 @@
> +/* Implementation of __nss_parse_line_result.
> + Copyright (C) 2020 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#include <nss_files.h>
> +
> +#include <assert.h>
> +#include <errno.h>
> +
> +int
> +__nss_parse_line_result (FILE *fp, off64_t offset, int parse_line_result)
> +{
> + assert (parse_line_result >= -1 && parse_line_result <= 1);
> +
> + 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 ();
> + }
> +}
> +libc_hidden_def (__nss_parse_line_result)
> diff --git a/nss/nss_readline.c b/nss/nss_readline.c
> new file mode 100644
> index 0000000000..44e0dd9319
> --- /dev/null
> +++ b/nss/nss_readline.c
> @@ -0,0 +1,99 @@
> +/* Read a line from an nss_files database file.
> + Copyright (C) 2020 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#include <nss_files.h>
> +
> +#include <ctype.h>
> +#include <errno.h>
> +#include <string.h>
> +
> +int
> +__nss_readline (FILE *fp, char *buf, size_t len, off64_t *poffset)
> +{
OK. Like this clean implementation.
> + /* We need space for at least one character, the line terminator,
> + and the NUL byte. */
> + if (len < 3)
> + {
> + *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;
> + if (*p == '\0' || *p == '#')
> + /* Skip empty lines and comments. */
> + continue;
> + if (p != buf)
> + memmove (buf, p, strlen (p));
> +
> + /* Return line to the caller. */
> + return 0;
> + }
> +}
> +libc_hidden_def (__nss_readline)
> +
> +int
> +__nss_readline_seek (FILE *fp, off64_t offset)
> +{
> + 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. */
OK.
> + fseterr_unlocked (fp);
> + __set_errno (ESPIPE);
> + return ESPIPE;
> + }
> + else
> + {
> + __set_errno (ERANGE);
> + return ERANGE;
> + }
> +}
>
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 07/11] grp: Implement fgetgrent_r using __nss_fgetent_r
2020-07-17 8:30 ` [PATCH 07/11] grp: Implement fgetgrent_r using __nss_fgetent_r Florian Weimer
@ 2020-07-21 3:28 ` Carlos O'Donell
0 siblings, 0 replies; 24+ messages in thread
From: Carlos O'Donell @ 2020-07-21 3:28 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
On 7/17/20 4:30 AM, Florian Weimer via Libc-alpha wrote:
OK for 2.32. Further cleanup of implmenetations using __nss_fgetent_r.
Tested-by: Carlos O'Donell <carlos@redhat.com>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> ---
> grp/fgetgrent_r.c | 54 ++++++-----------------------------------------
> 1 file changed, 6 insertions(+), 48 deletions(-)
>
> diff --git a/grp/fgetgrent_r.c b/grp/fgetgrent_r.c
> index 03daf4f295..b598584830 100644
> --- a/grp/fgetgrent_r.c
> +++ b/grp/fgetgrent_r.c
> @@ -20,10 +20,6 @@
> #include <grp.h>
> #include <stdio.h>
>
> -#include <libio/iolibio.h>
> -#define flockfile(s) _IO_flockfile (s)
> -#define funlockfile(s) _IO_funlockfile (s)
> -
> /* Define a line parsing function using the common code
> used in the nss_files module. */
>
> @@ -59,49 +55,11 @@ int
> __fgetgrent_r (FILE *stream, struct group *resbuf, char *buffer, size_t buflen,
> struct group **result)
> {
> - char *p;
> - int parse_result;
> -
> - flockfile (stream);
> - do
> - {
> - buffer[buflen - 1] = '\xff';
> - p = fgets_unlocked (buffer, buflen, stream);
> - if (__builtin_expect (p == NULL, 0) && feof_unlocked (stream))
> - {
> - funlockfile (stream);
> - *result = NULL;
> - __set_errno (ENOENT);
> - return errno;
> - }
> - if (__builtin_expect (p == NULL, 0) || buffer[buflen - 1] != '\xff')
> - {
> - funlockfile (stream);
> - *result = NULL;
> - __set_errno (ERANGE);
> - return errno;
> - }
> -
> - /* Skip leading blanks. */
> - 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_result = parse_line (p, resbuf,
> - (void *) buffer, buflen,
> - &errno)));
> -
> - funlockfile (stream);
> -
> - if (__builtin_expect (parse_result, 0) == -1)
> - {
> - /* The parser ran out of space. */
> - *result = NULL;
> - return errno;
> - }
> -
> - *result = resbuf;
> - return 0;
> + int ret = __nss_fgetent_r (stream, resbuf, buffer, buflen, parse_line);
> + if (ret == 0)
> + *result = resbuf;
> + else
> + *result = NULL;
> + return ret;
> }
> weak_alias (__fgetgrent_r, fgetgrent_r)
>
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 08/11] gshadow: Implement fgetsgent_r using __nss_fgetent_r (bug 20338)
2020-07-17 8:30 ` [PATCH 08/11] gshadow: Implement fgetsgent_r using __nss_fgetent_r (bug 20338) Florian Weimer
@ 2020-07-21 3:28 ` Carlos O'Donell
0 siblings, 0 replies; 24+ messages in thread
From: Carlos O'Donell @ 2020-07-21 3:28 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
On 7/17/20 4:30 AM, Florian Weimer via Libc-alpha wrote:
Huh, there isn't a man page for fgetsgent_r :}
OK for 2.32. Good cleanup and test. Implement with __nss_fgetent_r and
because we can use a FILE* we can test it with an arbitrary file.
Tested-by: Carlos O'Donell <carlos@redhat.com>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> ---
> gshadow/Makefile | 2 +-
> gshadow/fgetsgent_r.c | 41 ++------
> gshadow/tst-fgetsgent_r.c | 191 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 198 insertions(+), 36 deletions(-)
> create mode 100644 gshadow/tst-fgetsgent_r.c
>
> diff --git a/gshadow/Makefile b/gshadow/Makefile
> index 089aa6125f..32faf7a534 100644
> --- a/gshadow/Makefile
> +++ b/gshadow/Makefile
> @@ -26,7 +26,7 @@ headers = gshadow.h
> routines = getsgent getsgnam sgetsgent fgetsgent putsgent \
> getsgent_r getsgnam_r sgetsgent_r fgetsgent_r
>
> -tests = tst-gshadow tst-putsgent
> +tests = tst-gshadow tst-putsgent tst-fgetsgent_r
>
> CFLAGS-getsgent_r.c += -fexceptions
> CFLAGS-getsgent.c += -fexceptions
> diff --git a/gshadow/fgetsgent_r.c b/gshadow/fgetsgent_r.c
> index a7a1860c76..218206b4ac 100644
> --- a/gshadow/fgetsgent_r.c
> +++ b/gshadow/fgetsgent_r.c
> @@ -36,40 +36,11 @@ int
> __fgetsgent_r (FILE *stream, struct sgrp *resbuf, char *buffer, size_t buflen,
> struct sgrp **result)
> {
> - char *p;
> -
> - _IO_flockfile (stream);
> - do
> - {
> - 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. */
> - 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));
> -
> - _IO_funlockfile (stream);
> -
> - *result = resbuf;
> - return 0;
> + 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)
> diff --git a/gshadow/tst-fgetsgent_r.c b/gshadow/tst-fgetsgent_r.c
> new file mode 100644
> index 0000000000..780409afa9
> --- /dev/null
> +++ b/gshadow/tst-fgetsgent_r.c
> @@ -0,0 +1,191 @@
> +/* Test for fgetsgent_r and buffer sizes.
> + Copyright (C) 2020 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#include <array_length.h>
> +#include <errno.h>
> +#include <gshadow.h>
> +#include <stdbool.h>
> +#include <stdlib.h>
> +#include <support/check.h>
> +#include <support/support.h>
> +#include <support/temp_file.h>
> +#include <support/xmemstream.h>
> +#include <support/xstdio.h>
> +
> +/* Turn a parsed struct back into a line string. The returned string
> + should be freed. */
> +static char *
> +format_ent (const struct sgrp *e)
> +{
> + struct xmemstream stream;
> + xopen_memstream (&stream);
> + TEST_COMPARE (putsgent (e, stream.out), 0);
> + xfclose_memstream (&stream);
> + return stream.buffer;
> +}
> +
> +/* An entry in the input file along with the expected output. */
> +struct input
> +{
> + const char *line; /* Line in the file. */
> + const char *expected; /* Expected output. NULL if skipped. */
> +};
> +
> +const struct input inputs[] =
> + {
> + /* Regular entries. */
> + { "g1:x1::\n", "g1:x1::\n" },
> + { "g2:x2:a1:\n", "g2:x2:a1:\n" },
> + { "g3:x3:a2:u1\n", "g3:x3:a2:u1\n" },
> + { "g4:x4:a3,a4:u2,u3,u4\n", "g4:x4:a3,a4:u2,u3,u4\n" },
> +
> + /* Comments and empty lines. */
> + { "\n", NULL },
> + { " \n", NULL },
> + { "\t\n", NULL },
> + { "#g:x::\n", NULL },
> + { " #g:x::\n", NULL },
> + { "\t#g:x::\n", NULL },
> + { " \t#g:x::\n", NULL },
> +
> + /* Marker for synchronization. */
> + { "g5:x5::\n", "g5:x5::\n" },
> +
> + /* Leading whitespace. */
> + { " g6:x6::\n", "g6:x6::\n" },
> + { "\tg7:x7::\n", "g7:x7::\n" },
> +
> + /* This is expected to trigger buffer exhaustion during parsing
> + (bug 20338). */
> + {
> + "g8:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx:u5,u6,u7,u8,u9:\n",
> + "g8:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx:u5,u6,u7,u8,u9:\n",
> + },
> + {
> + "g9:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx::a5,a6,a7,a8,a9,a10\n",
> + "g9:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx::a5,a6,a7,a8,a9,a10\n",
> + },
> + };
> +
> +/* Writes the test data to a temporary file and returns its name. The
> + returned pointer should be freed. */
> +static char *
> +create_test_file (void)
> +{
> + char *path;
> + int fd = create_temp_file ("tst-fgetsgent_r-", &path);
> + FILE *fp = fdopen (fd, "w");
> + TEST_VERIFY_EXIT (fp != NULL);
> +
> + for (size_t i = 0; i < array_length (inputs); ++i)
> + fputs (inputs[i].line, fp);
> +
> + xfclose (fp);
> + return path;
> +}
> +
> +/* Read the test file with the indicated start buffer size. Return
> + true if the buffer size had to be increased during reading. */
> +static bool
> +run_test (const char *path, size_t buffer_size)
> +{
> + bool resized = false;
> + FILE *fp = xfopen (path, "r");
> +
> + /* This avoids repeated lseek system calls (bug 26257). */
> + TEST_COMPARE (fseeko64 (fp, 0, SEEK_SET), 0);
> +
> + size_t i = 0;
> + while (true)
> + {
> + /* Skip over unused expected entries. */
> + while (i < array_length (inputs) && inputs[i].expected == NULL)
> + ++i;
> +
> + /* Store the data on the heap, to help valgrind to detect
> + invalid accesses. */
> + struct sgrp *result_storage = xmalloc (sizeof (*result_storage));
> + char *buffer = xmalloc (buffer_size);
> + struct sgrp **result_pointer_storage
> + = xmalloc (sizeof (*result_pointer_storage));
> +
> + int ret = fgetsgent_r (fp, result_storage, buffer, buffer_size,
> + result_pointer_storage);
> + if (ret == 0)
> + {
> + TEST_VERIFY (*result_pointer_storage != NULL);
> + TEST_VERIFY (i < array_length (inputs));
> + if (*result_pointer_storage != NULL
> + && i < array_length (inputs))
> + {
> + char * actual = format_ent (*result_pointer_storage);
> + TEST_COMPARE_STRING (inputs[i].expected, actual);
> + free (actual);
> + ++i;
> + }
> + else
> + break;
> + }
> + else
> + {
> + TEST_VERIFY (*result_pointer_storage == NULL);
> + TEST_COMPARE (ret, errno);
> +
> + if (ret == ENOENT)
> + {
> + TEST_COMPARE (i, array_length (inputs));
> + free (result_pointer_storage);
> + free (buffer);
> + free (result_storage);
> + break;
> + }
> + else if (ret == ERANGE)
> + {
> + resized = true;
> + ++buffer_size;
> + }
> + else
> + FAIL_EXIT1 ("read failure: %m");
> + }
> +
> + free (result_pointer_storage);
> + free (buffer);
> + free (result_storage);
> + }
> +
> + return resized;
> +}
> +
> +static int
> +do_test (void)
> +{
> + char *path = create_test_file ();
> +
> + for (size_t buffer_size = 3; ; ++buffer_size)
> + {
> + bool resized = run_test (path, buffer_size);
> + if (!resized)
> + break;
> + }
> +
> + free (path);
> +
> + return 0;
> +}
> +
> +#include <support/test-driver.c>
>
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 09/11] pwd: Implement fgetpwent_r using __nss_fgetent_r
2020-07-17 8:30 ` [PATCH 09/11] pwd: Implement fgetpwent_r using __nss_fgetent_r Florian Weimer
@ 2020-07-21 3:28 ` Carlos O'Donell
0 siblings, 0 replies; 24+ messages in thread
From: Carlos O'Donell @ 2020-07-21 3:28 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
On 7/17/20 4:30 AM, Florian Weimer via Libc-alpha wrote:
OK for 2.32
Tested-by: Carlos O'Donell <carlos@redhat.com>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> ---
> pwd/fgetpwent_r.c | 43 ++++++-------------------------------------
> 1 file changed, 6 insertions(+), 37 deletions(-)
>
> diff --git a/pwd/fgetpwent_r.c b/pwd/fgetpwent_r.c
> index 26b0ee52aa..0a2e4a55e2 100644
> --- a/pwd/fgetpwent_r.c
> +++ b/pwd/fgetpwent_r.c
> @@ -20,9 +20,6 @@
> #include <stdio.h>
> #include <pwd.h>
>
> -#define flockfile(s) _IO_flockfile (s)
> -#define funlockfile(s) _IO_funlockfile (s)
> -
> /* Define a line parsing function using the common code
> used in the nss_files module. */
>
> @@ -72,39 +69,11 @@ int
> __fgetpwent_r (FILE *stream, struct passwd *resbuf, char *buffer,
> size_t buflen, struct passwd **result)
> {
> - char *p;
> -
> - flockfile (stream);
> - do
> - {
> - buffer[buflen - 1] = '\xff';
> - p = fgets_unlocked (buffer, buflen, stream);
> - if (p == NULL && feof_unlocked (stream))
> - {
> - funlockfile (stream);
> - *result = NULL;
> - __set_errno (ENOENT);
> - return errno;
> - }
> - if (p == NULL || buffer[buflen - 1] != '\xff')
> - {
> - funlockfile (stream);
> - *result = NULL;
> - __set_errno (ERANGE);
> - return errno;
> - }
> -
> - /* Skip leading blanks. */
> - 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 (p, resbuf, (void *) buffer, buflen, &errno));
> -
> - funlockfile (stream);
> -
> - *result = resbuf;
> - return 0;
> + int ret = __nss_fgetent_r (stream, resbuf, buffer, buflen, parse_line);
> + if (ret == 0)
> + *result = resbuf;
> + else
> + *result = NULL;
> + return ret;
> }
> weak_alias (__fgetpwent_r, fgetpwent_r)
>
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 10/11] shadow: Implement fgetspent_r using __nss_fgetent_r
2020-07-17 8:30 ` [PATCH 10/11] shadow: Implement fgetspent_r " Florian Weimer
@ 2020-07-21 3:28 ` Carlos O'Donell
0 siblings, 0 replies; 24+ messages in thread
From: Carlos O'Donell @ 2020-07-21 3:28 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
On 7/17/20 4:30 AM, Florian Weimer via Libc-alpha wrote:
OK for 2.32.
Tested-by: Carlos O'Donell <carlos@redhat.com>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> ---
> gshadow/tst-fgetsgent_r.c | 1 +
> shadow/fgetspent_r.c | 43 ++++++---------------------------------
> 2 files changed, 7 insertions(+), 37 deletions(-)
>
> diff --git a/gshadow/tst-fgetsgent_r.c b/gshadow/tst-fgetsgent_r.c
> index 780409afa9..f81e09b362 100644
> --- a/gshadow/tst-fgetsgent_r.c
> +++ b/gshadow/tst-fgetsgent_r.c
> @@ -168,6 +168,7 @@ run_test (const char *path, size_t buffer_size)
> free (result_storage);
> }
>
> + xfclose (fp);
> return resized;
> }
>
> diff --git a/shadow/fgetspent_r.c b/shadow/fgetspent_r.c
> index da35846df0..d4d4e52f6a 100644
> --- a/shadow/fgetspent_r.c
> +++ b/shadow/fgetspent_r.c
> @@ -20,9 +20,6 @@
> #include <shadow.h>
> #include <stdio.h>
>
> -#define flockfile(s) _IO_flockfile (s)
> -#define funlockfile(s) _IO_funlockfile (s)
> -
> /* Define a line parsing function using the common code
> used in the nss_files module. */
>
> @@ -39,39 +36,11 @@ int
> __fgetspent_r (FILE *stream, struct spwd *resbuf, char *buffer, size_t buflen,
> struct spwd **result)
> {
> - char *p;
> -
> - flockfile (stream);
> - do
> - {
> - buffer[buflen - 1] = '\xff';
> - p = fgets_unlocked (buffer, buflen, stream);
> - if (p == NULL && feof_unlocked (stream))
> - {
> - funlockfile (stream);
> - *result = NULL;
> - __set_errno (ENOENT);
> - return errno;
> - }
> - if (p == NULL || buffer[buflen - 1] != '\xff')
> - {
> - funlockfile (stream);
> - *result = NULL;
> - __set_errno (ERANGE);
> - return errno;
> - }
> -
> - /* Skip leading blanks. */
> - 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, NULL, 0, &errno));
> -
> - funlockfile (stream);
> -
> - *result = resbuf;
> - return 0;
> + int ret = __nss_fgetent_r (stream, resbuf, buffer, buflen, parse_line);
> + if (ret == 0)
> + *result = resbuf;
> + else
> + *result = NULL;
> + return ret;
> }
> weak_alias (__fgetspent_r, fgetspent_r)
>
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 11/11] libio: Remove __libc_readline_unlocked
2020-07-17 8:31 ` [PATCH 11/11] libio: Remove __libc_readline_unlocked Florian Weimer
@ 2020-07-21 3:28 ` Carlos O'Donell
0 siblings, 0 replies; 24+ messages in thread
From: Carlos O'Donell @ 2020-07-21 3:28 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
On 7/17/20 4:31 AM, Florian Weimer via Libc-alpha wrote:
> __nss_readline supersedes it. This reverts part of commit
> 3f5e3f5d066dcffb80af48ae2cf35a01a85a8f10 ("libio: Implement
> internal function __libc_readline_unlocked"). The internal
> aliases __fseeko64 and __ftello64 are preserved because
> they are needed by __nss_readline as well.
OK for 2.32.
Yes, __nss_readline supersedes __libc_readline_unlocked, and
you have used __nss_readline to fix a number of consistency
issues (and bug 20338), so we know semantically it's a better
fit. The alternative would have been to improve __libc_readline_unlocked
but it's less specialized and so would have required more boiler-plate
per use and I like the cleanup that the slightly more specialized
__nss_readline provides.
Tested-by: Carlos O'Donell <carlos@redhat.com>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> ---
> include/stdio.h | 13 ---
> libio/Makefile | 4 +-
> libio/Versions | 1 -
> libio/readline.c | 170 -------------------------------
> libio/tst-readline.c | 237 -------------------------------------------
> 5 files changed, 2 insertions(+), 423 deletions(-)
> delete mode 100644 libio/readline.c
> delete mode 100644 libio/tst-readline.c
>
> diff --git a/include/stdio.h b/include/stdio.h
> index ede0f3d444..2e0dc80c16 100644
> --- a/include/stdio.h
> +++ b/include/stdio.h
> @@ -179,19 +179,6 @@ int __vfxprintf (FILE *__fp, const char *__fmt, __gnuc_va_list,
> unsigned int)
> attribute_hidden;
>
> -/* Read the next line from FP into BUFFER, of LENGTH bytes. LINE will
> - include the line terminator and a NUL terminator. On success,
> - return the length of the line, including the line terminator, but
> - excluding the NUL termintor. On EOF, return zero and write a NUL
> - terminator. On error, return -1 and set errno. If the total byte
> - count (line and both terminators) exceeds LENGTH, return -1 and set
> - errno to ERANGE (but do not mark the stream as failed).
> -
> - The behavior is undefined if FP is not seekable, or if the stream
> - is already in an error state. */
> -ssize_t __libc_readline_unlocked (FILE *fp, char *buffer, size_t length);
> -libc_hidden_proto (__libc_readline_unlocked);
OK.
> -
> extern const char *const _sys_errlist_internal[] attribute_hidden;
> extern const char *__get_errlist (int) attribute_hidden;
> extern const char *__get_errname (int) attribute_hidden;
> diff --git a/libio/Makefile b/libio/Makefile
> index 926df1870b..4f0f777bbb 100644
> --- a/libio/Makefile
> +++ b/libio/Makefile
> @@ -49,7 +49,7 @@ routines := \
> __fbufsize __freading __fwriting __freadable __fwritable __flbf \
> __fpurge __fpending __fsetlocking \
> \
> - libc_fatal fmemopen oldfmemopen vtables readline
> + libc_fatal fmemopen oldfmemopen vtables
OK.
>
> tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc \
> tst_wprintf2 tst-widetext test-fmemopen tst-ext tst-ext2 \
> @@ -68,7 +68,7 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc \
> tst-sprintf-ub tst-sprintf-chk-ub tst-bz24051 tst-bz24153 \
> tst-wfile-sync
>
> -tests-internal = tst-vtables tst-vtables-interposed tst-readline
> +tests-internal = tst-vtables tst-vtables-interposed
>
OK.
> ifeq (yes,$(build-shared))
> # Add test-fopenloc only if shared library is enabled since it depends on
> diff --git a/libio/Versions b/libio/Versions
> index acb896afa9..6f1ab96100 100644
> --- a/libio/Versions
> +++ b/libio/Versions
> @@ -161,6 +161,5 @@ libc {
>
> __fseeko64;
> __ftello64;
> - __libc_readline_unlocked;
OK.
> }
> }
> diff --git a/libio/readline.c b/libio/readline.c
> deleted file mode 100644
> index 99fecc5825..0000000000
> --- a/libio/readline.c
> +++ /dev/null
> @@ -1,170 +0,0 @@
> -/* fgets with ERANGE error reporting and size_t buffer length.
> - Copyright (C) 2018-2020 Free Software Foundation, Inc.
> - This file is part of the GNU C Library.
> -
> - The GNU C Library is free software; you can redistribute it and/or
> - modify it under the terms of the GNU Lesser General Public
> - License as published by the Free Software Foundation; either
> - version 2.1 of the License, or (at your option) any later version.
> -
> - The GNU C Library is distributed in the hope that it will be useful,
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> - Lesser General Public License for more details.
> -
> - You should have received a copy of the GNU Lesser General Public
> - License along with the GNU C Library; if not, see
> - <https://www.gnu.org/licenses/>. */
> -
> -#include <assert.h>
> -#include <errno.h>
> -#include <stdio.h>
> -#include <string.h>
> -
> -#include "libioP.h"
> -
> -/* Return -1 and set errno to EINVAL if it is ERANGE. */
> -static ssize_t
> -fail_no_erange (void)
> -{
> - if (errno == ERANGE)
> - __set_errno (EINVAL);
> - return -1;
> -}
> -
> -/* Slow path for reading the line. Called with no data in the stream
> - read buffer. Write data to [BUFFER, BUFFER_END). */
> -static ssize_t
> -readline_slow (FILE *fp, char *buffer, char *buffer_end)
> -{
> - char *start = buffer;
> -
> - while (buffer < buffer_end)
> - {
> - if (__underflow (fp) == EOF)
> - {
> - if (_IO_ferror_unlocked (fp))
> - /* If the EOF was caused by a read error, report it. */
> - return fail_no_erange ();
> - *buffer = '\0';
> - /* Do not include the null terminator. */
> - return buffer - start;
> - }
> -
> - /* __underflow has filled the buffer. */
> - char *readptr = fp->_IO_read_ptr;
> - ssize_t readlen = fp->_IO_read_end - readptr;
> - /* Make sure that __underflow really has acquired some data. */
> - assert (readlen > 0);
> - char *pnl = memchr (readptr, '\n', readlen);
> - if (pnl != NULL)
> - {
> - /* We found the terminator. */
> - size_t line_length = pnl - readptr;
> - if (line_length + 2 > buffer_end - buffer)
> - /* Not enough room in the caller-supplied buffer. */
> - break;
> - memcpy (buffer, readptr, line_length + 1);
> - buffer[line_length + 1] = '\0';
> - fp->_IO_read_ptr = pnl + 1;
> - /* Do not include the null terminator. */
> - return buffer - start + line_length + 1;
> - }
> -
> - if (readlen >= buffer_end - buffer)
> - /* Not enough room in the caller-supplied buffer. */
> - break;
> -
> - /* Save and consume the stream buffer. */
> - memcpy (buffer, readptr, readlen);
> - fp->_IO_read_ptr = fp->_IO_read_end;
> - buffer += readlen;
> - }
> -
> - /* The line does not fit into the buffer. */
> - __set_errno (ERANGE);
> - return -1;
> -}
> -
> -ssize_t
> -__libc_readline_unlocked (FILE *fp, char *buffer, size_t buffer_length)
> -{
> - char *buffer_end = buffer + buffer_length;
> -
> - /* Orient the stream. */
> - if (__builtin_expect (fp->_mode, -1) == 0)
> - _IO_fwide (fp, -1);
> -
> - /* Fast path: The line terminator is found in the buffer. */
> - char *readptr = fp->_IO_read_ptr;
> - ssize_t readlen = fp->_IO_read_end - readptr;
> - off64_t start_offset; /* File offset before reading anything. */
> - if (readlen > 0)
> - {
> - char *pnl = memchr (readptr, '\n', readlen);
> - if (pnl != NULL)
> - {
> - size_t line_length = pnl - readptr;
> - /* Account for line and null terminators. */
> - if (line_length + 2 > buffer_length)
> - {
> - __set_errno (ERANGE);
> - return -1;
> - }
> - memcpy (buffer, readptr, line_length + 1);
> - buffer[line_length + 1] = '\0';
> - /* Consume the entire line. */
> - fp->_IO_read_ptr = pnl + 1;
> - return line_length + 1;
> - }
> -
> - /* If the buffer does not have enough space for what is pending
> - in the stream (plus a NUL terminator), the buffer is too
> - small. */
> - if (readlen + 1 > buffer_length)
> - {
> - __set_errno (ERANGE);
> - return -1;
> - }
> -
> - /* End of line not found. We need all the buffered data. Fall
> - through to the slow path. */
> - memcpy (buffer, readptr, readlen);
> - buffer += readlen;
> - /* The original length is invalid after this point. Use
> - buffer_end instead. */
> -#pragma GCC poison buffer_length
> - /* Read the old offset before updating the read pointer. */
> - start_offset = __ftello64 (fp);
> - fp->_IO_read_ptr = fp->_IO_read_end;
> - }
> - else
> - {
> - readlen = 0;
> - start_offset = __ftello64 (fp);
> - }
> -
> - /* Slow path: Read more data from the underlying file. We need to
> - restore the file pointer if the buffer is too small. First,
> - check if the __ftello64 call above failed. */
> - if (start_offset < 0)
> - return fail_no_erange ();
> -
> - ssize_t result = readline_slow (fp, buffer, buffer_end);
> - if (result < 0)
> - {
> - if (errno == ERANGE)
> - {
> - /* Restore the file pointer so that the caller may read the
> - same line again. */
> - if (__fseeko64 (fp, start_offset, SEEK_SET) < 0)
> - return fail_no_erange ();
> - __set_errno (ERANGE);
> - }
> - /* Do not restore the file position on other errors; it is
> - likely that the __fseeko64 call would fail, too. */
> - return -1;
> - }
> - return readlen + result;
> -}
> -libc_hidden_def (__libc_readline_unlocked)
> diff --git a/libio/tst-readline.c b/libio/tst-readline.c
> deleted file mode 100644
> index 40663ed77f..0000000000
> --- a/libio/tst-readline.c
> +++ /dev/null
> @@ -1,237 +0,0 @@
> -/* Test the __libc_readline_unlocked function.
> - Copyright (C) 2018-2020 Free Software Foundation, Inc.
> - This file is part of the GNU C Library.
> -
> - The GNU C Library is free software; you can redistribute it and/or
> - modify it under the terms of the GNU Lesser General Public
> - License as published by the Free Software Foundation; either
> - version 2.1 of the License, or (at your option) any later version.
> -
> - The GNU C Library is distributed in the hope that it will be useful,
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> - Lesser General Public License for more details.
> -
> - You should have received a copy of the GNU Lesser General Public
> - License along with the GNU C Library; if not, see
> - <https://www.gnu.org/licenses/>. */
> -
> -/* Exercise __libc_readline_unlocked with various combinations of line
> - lengths, stdio buffer sizes, and line read buffer sizes. */
> -
> -#include <errno.h>
> -#include <stdbool.h>
> -#include <stdio.h>
> -#include <string.h>
> -#include <support/check.h>
> -#include <support/support.h>
> -#include <support/temp_file.h>
> -#include <support/test-driver.h>
> -#include <support/xmemstream.h>
> -#include <support/xstdio.h>
> -#include <support/xunistd.h>
> -
> -enum
> - {
> - maximum_line_length = 7,
> - number_of_lines = 3,
> - };
> -
> -/* -1: Do not set buffer size. 0: unbuffered. Otherwise, use this as
> - the size of the buffer. */
> -static int buffer_size;
> -
> -/* These size of the buffer used for reading. Must be at least 2. */
> -static int read_size;
> -
> -/* If a read files with ERANGE, increase the buffer size by this
> - amount. Must be positive. */
> -static int read_size_increment;
> -
> -/* If non-zero, do not reset the read size after an ERANGE error. */
> -static int read_size_preserve;
> -
> -/* If non-zero, no '\n' at the end of the file. */
> -static int no_newline_at_eof;
> -
> -/* Length of the line, or -1 if the line is not present. */
> -static int line_lengths[number_of_lines];
> -
> -/* The name of the test file. */
> -static char *test_file_path;
> -
> -/* The contents of the test file. */
> -static char expected_contents[(maximum_line_length + 2) * number_of_lines + 1];
> -static size_t expected_length;
> -
> -/* Returns a random byte which is not zero or the line terminator. */
> -static char
> -random_char (void)
> -{
> - static unsigned int rand_state = 1;
> - while (true)
> - {
> - char result = rand_r (&rand_state) >> 16;
> - if (result != 0 && result != '\n')
> - return result;
> - }
> -}
> -
> -/* Create the test file. */
> -static void
> -prepare (int argc, char **argv)
> -{
> - int fd = create_temp_file ("tst-readline-", &test_file_path);
> - TEST_VERIFY_EXIT (fd >= 0);
> - xclose (fd);
> -}
> -
> -/* Prepare the test file. Return false if the test parameters are
> - incongruent and the test should be skipped. */
> -static bool
> -write_test_file (void)
> -{
> - expected_length = 0;
> - char *p = expected_contents;
> - for (int lineno = 0; lineno < number_of_lines; ++lineno)
> - for (int i = 0; i < line_lengths[lineno]; ++i)
> - *p++ = random_char ();
> - expected_length = p - &expected_contents[0];
> - if (no_newline_at_eof)
> - {
> - if (expected_length == 0)
> - return false;
> - --expected_length;
> - --p;
> - }
> - if (test_verbose > 0)
> - {
> - printf ("info: writing test file of %zu bytes:\n", expected_length);
> - for (int i = 0; i < number_of_lines; ++i)
> - printf (" line %d: %d\n", i, line_lengths[i]);
> - if (no_newline_at_eof)
> - puts (" (no newline at EOF)");
> - }
> - TEST_VERIFY_EXIT (expected_length < sizeof (expected_contents));
> - *p++ = '\0';
> - support_write_file_string (test_file_path, expected_contents);
> - return true;
> -}
> -
> -/* Run a single test (a combination of a test file and read
> - parameters). */
> -static void
> -run_test (void)
> -{
> - TEST_VERIFY_EXIT (read_size_increment > 0);
> - if (test_verbose > 0)
> - {
> - printf ("info: running test: buffer_size=%d read_size=%d\n"
> - " read_size_increment=%d read_size_preserve=%d\n",
> - buffer_size, read_size, read_size_increment, read_size_preserve);
> - }
> -
> - struct xmemstream result;
> - xopen_memstream (&result);
> -
> - FILE *fp = xfopen (test_file_path, "rce");
> - char *fp_buffer = NULL;
> - if (buffer_size == 0)
> - TEST_VERIFY_EXIT (setvbuf (fp, NULL, _IONBF, 0) == 0);
> - if (buffer_size > 0)
> - {
> - fp_buffer = xmalloc (buffer_size);
> - TEST_VERIFY_EXIT (setvbuf (fp, fp_buffer, _IOFBF, buffer_size) == 0);
> - }
> -
> - char *line_buffer = xmalloc (read_size);
> - size_t line_buffer_size = read_size;
> -
> - while (true)
> - {
> - ssize_t ret = __libc_readline_unlocked
> - (fp, line_buffer, line_buffer_size);
> - if (ret < 0)
> - {
> - TEST_VERIFY (ret == -1);
> - if (errno != ERANGE)
> - FAIL_EXIT1 ("__libc_readline_unlocked: %m");
> - line_buffer_size += read_size_increment;
> - free (line_buffer);
> - line_buffer = xmalloc (line_buffer_size);
> - /* Try reading this line again. */
> - }
> - else if (ret == 0)
> - break;
> - else
> - {
> - /* A line has been read. Save it. */
> - TEST_VERIFY (ret == strlen (line_buffer));
> - const char *pnl = strchr (line_buffer, '\n');
> - /* If there is a \n, it must be at the end. */
> - TEST_VERIFY (pnl == NULL || pnl == line_buffer + ret - 1);
> - fputs (line_buffer, result.out);
> -
> - /* Restore the original read size if required. */
> - if (line_buffer_size > read_size && !read_size_preserve)
> - {
> - line_buffer_size = read_size;
> - free (line_buffer);
> - line_buffer = xmalloc (line_buffer_size);
> - }
> - }
> - }
> -
> - xfclose (fp);
> - free (fp_buffer);
> - free (line_buffer);
> -
> - xfclose_memstream (&result);
> - TEST_VERIFY (result.length == expected_length);
> - TEST_VERIFY (strcmp (result.buffer, expected_contents) == 0);
> - if (test_verbose > 0)
> - {
> - printf ("info: expected (%zu): [[%s]]\n",
> - expected_length, expected_contents);
> - printf ("info: actual (%zu): [[%s]]\n", result.length, result.buffer);
> - }
> - free (result.buffer);
> -}
> -
> -/* Test one test file with multiple read parameters. */
> -static void
> -test_one_file (void)
> -{
> - for (buffer_size = -1; buffer_size <= maximum_line_length + 1; ++buffer_size)
> - for (read_size = 2; read_size <= maximum_line_length + 2; ++read_size)
> - for (read_size_increment = 1; read_size_increment <= 4;
> - ++read_size_increment)
> - for (read_size_preserve = 0; read_size_preserve < 2;
> - ++read_size_preserve)
> - run_test ();
> -}
> -
> -
> -static int
> -do_test (void)
> -{
> - /* Set up the test file contents. */
> - for (line_lengths[0] = -1; line_lengths[0] <= maximum_line_length;
> - ++line_lengths[0])
> - for (line_lengths[1] = -1; line_lengths[1] <= maximum_line_length;
> - ++line_lengths[1])
> - for (line_lengths[2] = -1; line_lengths[2] <= maximum_line_length;
> - ++line_lengths[2])
> - for (no_newline_at_eof = 0; no_newline_at_eof < 2; ++no_newline_at_eof)
> - {
> - if (!write_test_file ())
> - continue;
> - test_one_file ();
> - }
> - free (test_file_path);
> - return 0;
> -}
> -
> -#define TIMEOUT 100
> -#define PREPARE prepare
> -#include <support/test-driver.c>
OK.
>
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2020-07-21 3:28 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-17 8:29 [PATCH 00/11] Fix fgetsgent_r data corruption bug (20338) Florian Weimer
2020-07-17 8:30 ` [PATCH 01/11] nss_files: Consolidate file opening in __nss_files_fopen Florian Weimer
2020-07-21 3:27 ` Carlos O'Donell
2020-07-17 8:30 ` [PATCH 02/11] nss_compat: Do not use mmap to read database files (bug 26258) Florian Weimer
2020-07-21 3:27 ` Carlos O'Donell
2020-07-17 8:30 ` [PATCH 03/11] nss_files: Consolidate line parse declarations in <nss_files.h> Florian Weimer
2020-07-21 3:27 ` Carlos O'Donell
2020-07-17 8:30 ` [PATCH 04/11] nss_files: Use generic result pointer in parse_line Florian Weimer
2020-07-21 3:27 ` Carlos O'Donell
2020-07-17 8:30 ` [PATCH 05/11] libio: Add fseterr_unlocked for internal use Florian Weimer
2020-07-21 3:27 ` Carlos O'Donell
2020-07-17 8:30 ` [PATCH 06/11] nss: Add __nss_fgetent_r Florian Weimer
2020-07-21 3:27 ` Carlos O'Donell
2020-07-17 8:30 ` [PATCH 07/11] grp: Implement fgetgrent_r using __nss_fgetent_r Florian Weimer
2020-07-21 3:28 ` Carlos O'Donell
2020-07-17 8:30 ` [PATCH 08/11] gshadow: Implement fgetsgent_r using __nss_fgetent_r (bug 20338) Florian Weimer
2020-07-21 3:28 ` Carlos O'Donell
2020-07-17 8:30 ` [PATCH 09/11] pwd: Implement fgetpwent_r using __nss_fgetent_r Florian Weimer
2020-07-21 3:28 ` Carlos O'Donell
2020-07-17 8:30 ` [PATCH 10/11] shadow: Implement fgetspent_r " Florian Weimer
2020-07-21 3:28 ` Carlos O'Donell
2020-07-17 8:31 ` [PATCH 11/11] libio: Remove __libc_readline_unlocked Florian Weimer
2020-07-21 3:28 ` Carlos O'Donell
2020-07-21 3:27 ` [PATCH 00/11] Fix fgetsgent_r data corruption bug (20338) Carlos O'Donell
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 read-only IMAP folder(s) and NNTP newsgroup(s).