Re: [PATCH 02/11] nss_compat: Do not use mmap to read database files (bug 26258)

[Carlos O'Donell <carlos@redhat.com>]

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.