[01/11] nss_files: Consolidate file opening in __nss_files_fopen

Message ID b4abeec985014cd303462730a279b044a8c79e05.1594974444.git.fweimer@redhat.com
State Committed
Headers
Series Fix fgetsgent_r data corruption bug (20338) |

Commit Message

Florian Weimer July 17, 2020, 8:30 a.m. UTC
  ---
 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
  

Comments

Carlos O'Donell July 21, 2020, 3:27 a.m. UTC | #1
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)
>
  

Patch

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)