picnoir/glibc
1
0
Fork 0
Code Issues Pull requests Packages Projects Releases Wiki Activity

login: Acquire write lock early in pututline [BZ #24882]

Browse source 
It has been reported that due to lack of fairness in POSIX file
locking, the current reader-to-writer lock upgrade can result in
lack of forward progress.  Acquiring the write lock directly
hopefully avoids this issue if there are only writers.

This also fixes bug 24882 due to the cache revalidation in
__libc_pututline.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>
Change-Id: I57e31ae30719e609a53505a0924dda101d46372e
This commit is contained in:
Florian Weimer 2019-11-07 18:15:18 +01:00
parent 4f4bb489e0
commit be6b16d975
3 changed files with 240 additions and 33 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
login/Makefile
View file

@ -44,7 +44,7 @@ subdir-dirs = programs
vpath %.c programs
tests := tst-utmp tst-utmpx tst-grantpt tst-ptsname tst-getlogin tst-updwtmpx \
tst-pututxline-lockfail
tst-pututxline-lockfail tst-pututxline-cache
# Build the -lutil library with these extra functions.
extra-libs := libutil
@ -74,3 +74,4 @@ $(inst_libexecdir)/pt_chown: $(objpfx)pt_chown $(+force)
-$(INSTALL_PROGRAM) -m 4755 -o root $< $@
$(objpfx)tst-pututxline-lockfail: $(shared-thread-library)
$(objpfx)tst-pututxline-cache: $(shared-thread-library)

193
login/tst-pututxline-cache.c Normal file
View file

@ -0,0 +1,193 @@
/* Test case for cache invalidation after concurrent write (bug 24882).
Copyright (C) 2019 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; see the file COPYING.LIB. If
not, see <http://www.gnu.org/licenses/>. */
/* This test writes an entry to the utmpx file, reads it (so that it
is cached) in process1, and overwrites the same entry in process2
with something that does not match the search criteria. At this
point, the cache of the first process is stale, and when process1
attempts to write a new record which would have gone to the same
place (as indicated by the cache), it needs to realize that it has
to pick a different slot because the old slot is now used for
something else. */
#include <errno.h>
#include <stdlib.h>
#include <string.h>
#include <support/check.h>
#include <support/namespace.h>
#include <support/support.h>
#include <support/temp_file.h>
#include <support/xthread.h>
#include <support/xunistd.h>
#include <utmp.h>
#include <utmpx.h>
/* Set to the path of the utmp file. */
static char *utmp_file;
/* Used to synchronize the subprocesses. The barrier itself is
allocated in shared memory. */
static pthread_barrier_t *barrier;
/* setutxent with error checking. */
static void
xsetutxent (void)
{
errno = 0;
setutxent ();
TEST_COMPARE (errno, 0);
}
/* getutxent with error checking. */
static struct utmpx *
xgetutxent (void)
{
errno = 0;
struct utmpx *result = getutxent ();
if (result == NULL)
FAIL_EXIT1 ("getutxent: %m");
return result;
}
static void
put_entry (const char *id, pid_t pid, const char *user, const char *line)
{
struct utmpx ut =
{
.ut_type = LOGIN_PROCESS,
.ut_pid = pid,
.ut_host = "localhost",
};
strcpy (ut.ut_id, id);
strncpy (ut.ut_user, user, sizeof (ut.ut_user));
strncpy (ut.ut_line, line, sizeof (ut.ut_line));
TEST_VERIFY (pututxline (&ut) != NULL);
}
/* Use two cooperating subprocesses to avoid issues related to
unlock-on-close semantics of POSIX advisory locks. */
static __attribute__ ((noreturn)) void
process1 (void)
{
TEST_COMPARE (utmpname (utmp_file), 0);
/* Create an entry. */
xsetutxent ();
put_entry ("1", 101, "root", "process1");
/* Retrieve the entry. This will fill the internal cache. */
{
errno = 0;
setutxent ();
TEST_COMPARE (errno, 0);
struct utmpx ut =
{
.ut_type = LOGIN_PROCESS,
.ut_line = "process1",
};
struct utmpx *result = getutxline (&ut);
if (result == NULL)
FAIL_EXIT1 ("getutxline (\"process1\"): %m");
TEST_COMPARE (result->ut_pid, 101);
}
/* Signal the other process to overwrite the entry. */
xpthread_barrier_wait (barrier);
/* Wait for the other process to complete the write operation. */
xpthread_barrier_wait (barrier);
/* Add another entry. Note: This time, there is no setutxent call. */
put_entry ("1", 103, "root", "process1");
_exit (0);
}
static void
process2 (void *closure)
{
/* Wait for the first process to write its entry. */
xpthread_barrier_wait (barrier);
/* Truncate the file. The glibc interface does not support
re-purposing records, but an external expiration mechanism may
trigger this. */
TEST_COMPARE (truncate64 (utmp_file, 0), 0);
/* Write the replacement entry. */
TEST_COMPARE (utmpname (utmp_file), 0);
xsetutxent ();
put_entry ("2", 102, "user", "process2");
/* Signal the other process that the entry has been replaced. */
xpthread_barrier_wait (barrier);
}
static int
do_test (void)
{
xclose (create_temp_file ("tst-tumpx-cache-write-", &utmp_file));
{
pthread_barrierattr_t attr;
xpthread_barrierattr_init (&attr);
xpthread_barrierattr_setpshared (&attr, PTHREAD_SCOPE_PROCESS);
barrier = support_shared_allocate (sizeof (*barrier));
xpthread_barrier_init (barrier, &attr, 2);
}
/* Run both subprocesses in parallel. */
{
pid_t pid1 = xfork ();
if (pid1 == 0)
process1 ();
support_isolate_in_subprocess (process2, NULL);
int status;
xwaitpid (pid1, &status, 0);
TEST_COMPARE (status, 0);
}
/* Check that the utmpx database contains the expected records. */
{
TEST_COMPARE (utmpname (utmp_file), 0);
xsetutxent ();
struct utmpx *ut = xgetutxent ();
TEST_COMPARE_STRING (ut->ut_id, "2");
TEST_COMPARE (ut->ut_pid, 102);
TEST_COMPARE_STRING (ut->ut_user, "user");
TEST_COMPARE_STRING (ut->ut_line, "process2");
ut = xgetutxent ();
TEST_COMPARE_STRING (ut->ut_id, "1");
TEST_COMPARE (ut->ut_pid, 103);
TEST_COMPARE_STRING (ut->ut_user, "root");
TEST_COMPARE_STRING (ut->ut_line, "process1");
if (getutxent () != NULL)
FAIL_EXIT1 ("additional utmpx entry");
}
xpthread_barrier_destroy (barrier);
support_shared_free (barrier);
free (utmp_file);
return 0;
}
#include <support/test-driver.c>

77
login/utmp_file.c
View file

@ -186,19 +186,11 @@ __libc_getutent_r (struct utmp *buffer, struct utmp **result)
/* Search for *ID, updating last_entry and file_offset. Return 0 on
success and -1 on failure. If the locking operation failed, write
true to *LOCK_FAILED. */
success and -1 on failure. Does not perform locking; for that see
internal_getut_r below. */
static int
internal_getut_r (const struct utmp *id, bool *lock_failed)
internal_getut_nolock (const struct utmp *id)
{
int result = -1;
if (try_file_lock (file_fd, F_RDLCK))
{
*lock_failed = true;
return -1;
}
if (id->ut_type == RUN_LVL || id->ut_type == BOOT_TIME
|| id->ut_type == OLD_TIME || id->ut_type == NEW_TIME)
{
@ -213,7 +205,7 @@ internal_getut_r (const struct utmp *id, bool *lock_failed)
{
__set_errno (ESRCH);
file_offset = -1l;
goto unlock_return;
return -1;
}
file_offset += sizeof (struct utmp);
@ -234,7 +226,7 @@ internal_getut_r (const struct utmp *id, bool *lock_failed)
{
__set_errno (ESRCH);
file_offset = -1l;
goto unlock_return;
return -1;
}
file_offset += sizeof (struct utmp);
@ -243,14 +235,25 @@ internal_getut_r (const struct utmp *id, bool *lock_failed)
}
}
result = 0;
unlock_return:
file_unlock (file_fd);
return result;
return 0;
}
/* Search for *ID, updating last_entry and file_offset. Return 0 on
success and -1 on failure. If the locking operation failed, write
true to *LOCK_FAILED. */
static int
internal_getut_r (const struct utmp *id, bool *lock_failed)
{
if (try_file_lock (file_fd, F_RDLCK))
{
*lock_failed = true;
return -1;
}
int result = internal_getut_nolock (id);
file_unlock (file_fd);
return result;
}
/* For implementing this function we don't use the getutent_r function
because we can avoid the reposition on every new entry this way. */
@ -279,7 +282,6 @@ __libc_getutid_r (const struct utmp *id, struct utmp *buffer,
return 0;
}
/* For implementing this function we don't use the getutent_r function
because we can avoid the reposition on every new entry this way. */
int
@ -336,7 +338,6 @@ __libc_pututline (const struct utmp *data)
return NULL;
struct utmp *pbuf;
int found;
if (! file_writable)
{
@ -358,7 +359,12 @@ __libc_pututline (const struct utmp *data)
file_writable = true;
}
/* Exclude other writers before validating the cache. */
if (try_file_lock (file_fd, F_WRLCK))
return NULL;
/* Find the correct place to insert the data. */
bool found = false;
if (file_offset > 0
&& ((last_entry.ut_type == data->ut_type
&& (last_entry.ut_type == RUN_LVL
@ -366,23 +372,30 @@ __libc_pututline (const struct utmp *data)
|| last_entry.ut_type == OLD_TIME
|| last_entry.ut_type == NEW_TIME))
|| __utmp_equal (&last_entry, data)))
found = 1;
else
{
bool lock_failed = false;
found = internal_getut_r (data, &lock_failed);
if (__builtin_expect (lock_failed, false))
if (__lseek64 (file_fd, file_offset, SEEK_SET) < 0)
{
__set_errno (EAGAIN);
file_unlock (file_fd);
return NULL;
}
if (__read_nocancel (file_fd, &last_entry, sizeof (last_entry))
!= sizeof (last_entry))
{
if (__lseek64 (file_fd, file_offset, SEEK_SET) < 0)
{
file_unlock (file_fd);
return NULL;
}
found = false;
}
else
found = __utmp_equal (&last_entry, data);
}
if (try_file_lock (file_fd, F_WRLCK))
return NULL;
if (!found)
found = internal_getut_nolock (data) >= 0;
if (found < 0)
if (!found)
{
/* We append the next entry. */
file_offset = __lseek64 (file_fd, 0, SEEK_END);
@ -411,7 +424,7 @@ __libc_pututline (const struct utmp *data)
{
/* If we appended a new record this is only partially written.
Remove it. */
if (found < 0)
if (!found)
(void) __ftruncate64 (file_fd, file_offset);
pbuf = NULL;
}