@@ -1,4 +1,4 @@
-/* Test the lock upgrade path in tst-pututxline.
+/* Test the lock path in putut{x}line.
Copyright (C) 2019-2020 Free Software Foundation, Inc.
This file is part of the GNU C Library.
@@ -16,20 +16,25 @@
License along with the GNU C Library; see the file COPYING.LIB. If
not, see <https://www.gnu.org/licenses/>. */
-/* pututxline upgrades the read lock on the file to a write lock.
- This test verifies that if the lock upgrade fails, the utmp
- subsystem remains in a consistent state, so that pututxline can be
- called again. */
+/* putut{x}line and updwtmp{x} do not lock the utmp{x} file, but rather a
+ specific file based on the database path. For non default files
+ set by utmpname is is the '/var/lock/`basename $path`.lock.
+ This test verifies that if the lock fails, the utmp subsystem remains
+ in a consistent state, so that putut{x}line can be called again. */
#include <errno.h>
#include <fcntl.h>
#include <stdlib.h>
+#include <stdio.h>
+#include <limits.h>
+#include <libgen.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 <support/xsignal.h>
#include <unistd.h>
#include <utmp.h>
#include <utmpx.h>
@@ -66,11 +71,15 @@ subprocess_create_entry (void *closure)
TEST_VERIFY (write_entry (101) != NULL);
}
-/* Acquire an advisory read lock on PATH. */
+/* Acquire an advisory read lock file for on PATH. */
__attribute__ ((noreturn)) static void
subprocess_lock_file (void)
{
- int fd = xopen (path, O_RDONLY, 0);
+ char lockpath[PATH_MAX];
+ snprintf (lockpath, sizeof (lockpath), "%s.lock", path);
+
+ int fd = xopen (lockpath, O_RDONLY | O_CREAT, 0644);
+ add_temp_file (lockpath);
struct flock64 fl =
{
@@ -101,6 +110,9 @@ subprocess_lock_file (void)
_exit (0);
}
+/* Do-nothing handler for locking timeout. */
+static void timeout_handler (int signum) {};
+
static int
do_test (void)
{
@@ -125,6 +137,20 @@ do_test (void)
/* Wait for the file locking to complete. */
xpthread_barrier_wait (barrier);
+ {
+ /* The 'subprocess_lock_file' creates and locks the file created by
+ utmpname pututxline, the SIGARLM forces the utmp lock to return
+ and the function call to fail. */
+
+ struct sigaction action;
+ action.sa_handler = timeout_handler;
+ sigemptyset (&action.sa_mask);
+ action.sa_flags = 0;
+ xsigaction (SIGALRM, &action, NULL);
+
+ alarm (5);
+ }
+
/* Try to add another entry. This attempt will fail, with EINTR or
EAGAIN. */
TEST_COMPARE (utmpname (path), 0);
@@ -5,6 +5,12 @@ touch 0664 /var/run/wtmp.v2
# Same for the old files as well
touch 0664 /var/run/utmp
touch 0664 /var/run/wtmp
+# For default UTMP_FILE we need to create the lock files
+mkdirp 0755 /var/lock
+touch 0664 /var/lock/utmp.v2.lock
+touch 0664 /var/lock/wtmp.v2.lock
+touch 0664 /var/lock/utmp.lock
+touch 0664 /var/lock/wtmp.lock
# Must run localedef as root to write into default paths.
su
@@ -2,6 +2,10 @@
mkdirp 0755 /var/run
touch 0664 /var/run/utmp.v2
touch 0664 /var/run/wtmp.v2
+# For default UTMP_FILE we need to create the lock files
+mkdirp 0755 /var/lock
+touch 0664 /var/lock/utmp.v2.lock
+touch 0664 /var/lock/wtmp.v2.lock
# Must run localedef as root to write into default paths.
su
@@ -20,6 +20,7 @@
#include <fcntl.h>
#include <stdbool.h>
#include <string.h>
+#include <stdio.h>
#include <sys/param.h>
#include <not-cancel.h>
@@ -121,62 +122,63 @@ matches_last_entry (enum operation_mode_t mode, short int type,
}
}
-/* Locking timeout. */
-#ifndef TIMEOUT
-# define TIMEOUT 10
-#endif
+/* Construct a lock file base on FILE depending of DEFAULT_DB: if true
+ the lock is constructed on /var/lock; otherwise is used the FILE
+ path itself. The lock file is also created if it does not exist if
+ DEFAULT_DB is false. */
+static int
+lock_write_file (const char *file, bool default_db)
+{
+ char path[PATH_MAX];
+ if (default_db)
+ __snprintf (path, sizeof (path), "/var/lock/%s.lock", __basename (file));
+ else
+ __snprintf (path, sizeof (path), "%s.lock", file);
-/* Do-nothing handler for locking timeout. */
-static void timeout_handler (int signum) {};
+ int flags = O_RDWR | O_LARGEFILE | O_CLOEXEC;
+ mode_t mode = 0644;
+ /* The errno need to reset if 'create_file' is set and the O_CREAT does not
+ fail. */
+ int saved_errno = errno;
+ int fd = __open_nocancel (path, flags, mode);
+ if (fd == -1 && errno == ENOENT && !default_db)
+ fd = __open_nocancel (path, flags | O_CREAT, mode);
+ if (fd == -1)
+ return -1;
+ __set_errno (saved_errno);
-/* try_file_lock (LOCKING, FD, TYPE) returns true if the locking
- operation failed and recovery needs to be performed.
+ struct flock64 fl =
+ {
+ .l_type = F_WRLCK,
+ .l_whence = SEEK_SET,
+ };
- file_unlock (FD) removes the lock (which must have been
- successfully acquired). */
+ if (__fcntl64_nocancel (fd, F_SETLKW, &fl) == -1)
+ {
+ __close_nocancel_nostatus (fd);
+ return -1;
+ }
+ return fd;
+}
-static bool
-try_file_lock (int fd, int type)
-{
- /* Cancel any existing alarm. */
- int old_timeout = alarm (0);
-
- /* Establish signal handler. */
- struct sigaction old_action;
- struct sigaction action;
- action.sa_handler = timeout_handler;
- __sigemptyset (&action.sa_mask);
- action.sa_flags = 0;
- __sigaction (SIGALRM, &action, &old_action);
-
- alarm (TIMEOUT);
-
- /* Try to get the lock. */
- struct flock64 fl =
- {
- .l_type = type,
- .l_whence = SEEK_SET,
- };
-
- bool status = __fcntl64_nocancel (fd, F_SETLKW, &fl) < 0;
- int saved_errno = errno;
-
- /* Reset the signal handler and alarm. We must reset the alarm
- before resetting the handler so our alarm does not generate a
- spurious SIGALRM seen by the user. However, we cannot just set
- the user's old alarm before restoring the handler, because then
- it's possible our handler could catch the user alarm's SIGARLM and
- then the user would never see the signal he expected. */
- alarm (0);
- __sigaction (SIGALRM, &old_action, NULL);
- if (old_timeout != 0)
- alarm (old_timeout);
+static void
+unlock_write_file (const char *file, int lockfd, bool default_db)
+{
+ __close_nocancel_nostatus (lockfd);
- __set_errno (saved_errno);
- return status;
+ char path[PATH_MAX];
+ __snprintf (path, sizeof (path), "/var/lock/%s.lock", basename (file));
+ if (! default_db)
+ {
+ /* Ignore error for the case the file does not exist. */
+ int saved_errno = errno;
+ __unlink (path);
+ __set_errno (saved_errno);
+ }
}
+
static void
file_unlock (int fd)
{
@@ -251,8 +253,7 @@ internal_getutent_r (enum operation_mode_t mode, void *buffer)
{
int saved_errno = errno;
- if (!maybe_setutent (mode)
- || try_file_lock (file_fd, F_RDLCK))
+ if (!maybe_setutent (mode))
return -1;
ssize_t nbytes = read_last_entry (mode);
@@ -304,9 +305,6 @@ static bool
internal_getut_r (enum operation_mode_t mode, short int type, const char *id,
const char *line)
{
- if (try_file_lock (file_fd, F_RDLCK))
- return false;
-
bool r = internal_getut_nolock (mode, type, id, line);
file_unlock (file_fd);
return r;
@@ -335,8 +333,7 @@ static int
internal_getutline_r (enum operation_mode_t mode, const char *line,
void *buffer)
{
- if (!maybe_setutent (mode)
- || try_file_lock (file_fd, F_RDLCK))
+ if (!maybe_setutent (mode))
return -1;
while (1)
@@ -375,13 +372,13 @@ internal_pututline (enum operation_mode_t mode, short int type,
if (!maybe_setutent (mode))
return false;
+ const char *file_name = mode == UTMP_TIME64
+ ? __libc_utmp_file_name
+ : utmp_file_name_time32 (__libc_utmp_file_name);
+
if (! file_writable)
{
/* We must make the file descriptor writable before going on. */
- const char *file_name = mode == UTMP_TIME64
- ? __libc_utmp_file_name
- : utmp_file_name_time32 (__libc_utmp_file_name);
-
int new_fd = __open_nocancel
(file_name, O_RDWR | O_LARGEFILE | O_CLOEXEC);
if (new_fd == -1)
@@ -396,34 +393,25 @@ internal_pututline (enum operation_mode_t mode, short int type,
file_writable = true;
}
- /* Exclude other writers before validating the cache. */
- if (try_file_lock (file_fd, F_WRLCK))
+ /* To avoid DOS when accessing the utmp{x} database for update, the lock
+ file should be accessible only by previleged users (BZ #24492). For non
+ default utmp{x} database the function tries to create the lock file. */
+ bool default_db = __libc_utmpname_mode == UTMPNAME_TIME64
+ || __libc_utmpname_mode == UTMPNAME_TIME32;
+ int lockfd = lock_write_file (file_name, default_db);
+ if (lockfd == -1)
return false;
/* Find the correct place to insert the data. */
const size_t utmp_size = last_entry_size (mode);
- bool found = false;
- if (matches_last_entry (mode, type, id, line))
- {
- /* Read back the entry under the write lock. */
- file_offset -= utmp_size;
- ssize_t nbytes = read_last_entry (mode);
- if (nbytes < 0)
- {
- file_unlock (file_fd);
- return false;
- }
-
- if (nbytes == 0)
- /* End of file reached. */
- found = false;
- else
- found = matches_last_entry (mode, type, id, line);
- }
+ bool ret = false;
- if (!found)
- /* Search forward for the entry. */
- found = internal_getut_nolock (mode, type, id, line);
+ if (matches_last_entry (mode, type, id, line))
+ /* Read back the entry under the write lock. */
+ file_offset -= utmp_size;
+ bool found = internal_getut_nolock (mode, type, id, line);
+ if (!found && errno != ESRCH)
+ goto internal_pututline_out;
off64_t write_offset;
if (!found)
@@ -444,31 +432,29 @@ internal_pututline (enum operation_mode_t mode, short int type,
ssize_t nbytes;
if (__lseek64 (file_fd, write_offset, SEEK_SET) < 0
|| (nbytes = __write_nocancel (file_fd, data, utmp_size)) < 0)
- {
- /* There is no need to recover the file position because all
- reads use pread64, and any future write is preceded by
- another seek. */
- file_unlock (file_fd);
- return false;
- }
+ /* There is no need to recover the file position because all reads use
+ pread64, and any future write is preceded by another seek. */
+ goto internal_pututline_out;
if (nbytes != utmp_size)
{
/* If we appended a new record this is only partially written.
Remove it. */
if (!found)
- (void) __ftruncate64 (file_fd, write_offset);
- file_unlock (file_fd);
+ __ftruncate64 (file_fd, write_offset);
/* Assume that the write failure was due to missing disk
space. */
__set_errno (ENOSPC);
- return false;
+ goto internal_pututline_out;
}
- file_unlock (file_fd);
file_offset = write_offset + utmp_size;
+ ret = true;
- return true;
+internal_pututline_out:
+ /* Release the write lock. */
+ unlock_write_file (file_name, lockfd, default_db);
+ return ret;
}
static int
@@ -484,7 +470,10 @@ internal_updwtmp (enum operation_mode_t mode, const char *file,
if (fd < 0)
return -1;
- if (try_file_lock (fd, F_WRLCK))
+ bool default_db = strcmp (file, _PATH_UTMP) == 0
+ || strcmp (file, _PATH_UTMP_BASE) == 0;
+ int lockfd = lock_write_file (file, default_db);
+ if (lockfd == -1)
{
__close_nocancel_nostatus (fd);
return -1;
@@ -514,9 +503,8 @@ internal_updwtmp (enum operation_mode_t mode, const char *file,
result = 0;
unlock_return:
- file_unlock (fd);
-
/* Close WTMP file. */
+ unlock_write_file (file, lockfd, default_db);
__close_nocancel_nostatus (fd);
return result;