[Bug nss/26713] New: nss_compat getpwuid_r uses thread unsafe fgets_unlocked

[Bug nss/26713] New: nss_compat getpwuid_r uses thread unsafe fgets_unlocked

[stolarek.marcin at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=26713

            Bug ID: 26713
           Summary: nss_compat getpwuid_r uses thread unsafe
                    fgets_unlocked
           Product: glibc
           Version: unspecified
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: nss
          Assignee: unassigned at sourceware dot org
          Reporter: stolarek.marcin at gmail dot com
  Target Milestone: ---

Created attachment 12892
  --> https://sourceware.org/bugzilla/attachment.cgi?id=12892&action=edit
backtrace of SIGBUS thread

Working on the case https://bugs.schedmd.com/show_bug.cgi?id=9827 where user
received a SIGBUS with si_code=2 on a thread executing getpwuid_r from
nss_compat I found that in compat-pwd.c function: internal_getpwuid_r glibc
makes use of fgets_unlocked which is a non-thread safe function.

The backtrace(attached) confirms that it was in this stack during the code
execution.

Shouldn't it be replaced by a standard thread safe fgets? Like:

>From 0703d715d090dfa7e710d1aa5e98ece8b8c258c5 Mon Sep 17 00:00:00 2001
From: Marcin Stolarek <stolarek.marcin@gmail.com>
Date: Wed, 7 Oct 2020 12:03:57 +0200
Subject: [PATCH 1/1] Replace fgets_unlocked with fgets in thread-safe function

internal_getpwuid_r is meant to be thread safe so it should not use any
non-thread safe function.
---
 nis/nss_compat/compat-pwd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nis/nss_compat/compat-pwd.c b/nis/nss_compat/compat-pwd.c
index 85b79e084e..6be5763641 100644
--- a/nis/nss_compat/compat-pwd.c
+++ b/nis/nss_compat/compat-pwd.c
@@ -944,7 +944,7 @@ internal_getpwuid_r (uid_t uid, struct passwd *result,
ent_t *ent,

          fgetpos (ent->stream, &pos);
          buffer[buflen - 1] = '\xff';
-         p = fgets_unlocked (buffer, buflen, ent->stream);
+         p = fgets (buffer, buflen, ent->stream);
          if (p == NULL && feof_unlocked (ent->stream))
            return NSS_STATUS_NOTFOUND;

-- 
2.17.1

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug nss/26713] nss_compat getpwuid_r uses thread unsafe fgets_unlocked

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=26713

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fweimer at redhat dot com

--- Comment #1 from Florian Weimer <fweimer at redhat dot com> ---
fgets_unlocked is thread-safe if access to the stream is synchronized
externally.

In nss_compat, this synchronization is performed using the global lock
variable.

I think the actual problem is that this lock is released before
internal_getpwuid_r is called in _nss_compat_getpwnam_r and
_nss_compat_getpwuid_r.

-- 
You are receiving this mail because:
You are on the CC list for the bug.