This is the mail archive of the
libc-hacker@sourceware.cygnus.com
mailing list for the glibc project.
Locking of UTMP-files?
- To: libc-hacker@cygnus.com
- Subject: Locking of UTMP-files?
- From: Mark Kettenis <kettenis@phys.uva.nl>
- Date: Mon, 18 May 1998 17:17:05 +0200 (MET DST)
Hi,
Some time ago, April 27, I sent a message to the list discussing a
patch to the UTMP-code to implement reliable locking of UTMP files.
Since I got no reaction, I assume that either nobody got it, or nobody
at that time had the time to take a look at it. Anyway, I append it
here again. Let me stress that the current code is broken, since file
locking is effectively disabled in the current code.
Mark
---------------------------------------------------------------------
Hi,
The following patch tries to implement a more robust way of locking
UTMP-like files. The idea is to use the F_SETLKW fcntl command all
over the place and install a SIGALRM signal handler that may interrupt
the system call. Now if someone holds a bogus read lock, pututline
won't hang, but will break the lock after a 1 second timeout. If
someone holds a bogus write lock (something that cannot easily be done
by a malicious user), all functions will break the lock after this
timeout. For programs reading a lot of UTMP entries this causes a
rather big slowdown, but at least the program will not hang.
However i am not entirely sure whether this approach is entirely
thread-safe. I think I correctly save an restore a previously
installed handler and alarm, but the interaction between threads and
signal handling is not entirely clear to me. Perhaps someone with
some more knowledge could take a look?
Mark
1998-04-25 Mark Kettenis <kettenis@phys.uva.nl>
* sysdeps/generic/utmp_file.c (TIMEOUT): New macro.
(timeout_handler): New function.
(LOCK_FILE, UNLOCK_FILE): New macros.
Implement file locking with timeout.
(getutent_r_file, internal_getut_r, getutline_r_file,
pututline_file, updwtmp_file): Use LOCK_FILE and UNLOCK_FILE for
locking.
===================================================================
RCS file: libc/sysdeps/generic/utmp_file.c,v
retrieving revision 1.1
diff -u -r1.1 libc/sysdeps/generic/utmp_file.c
--- libc/sysdeps/generic/utmp_file.c 1998/04/25 14:18:46 1.1
+++ libc/sysdeps/generic/utmp_file.c 1998/04/25 14:40:36
@@ -21,6 +21,7 @@
#include <assert.h>
#include <errno.h>
#include <fcntl.h>
+#include <signal.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
@@ -37,6 +38,48 @@
static struct utmp last_entry;
+/* Locking timeout. */
+#ifndef TIMEOUT
+# define TIMEOUT 1
+#endif
+
+/* Do-nothing handler for locking timeout. */
+static void timeout_handler (int signum) {};
+
+#define LOCK_FILE(fd, type) \
+{ \
+ struct flock fl; \
+ struct sigaction action, old_action; \
+ unsigned int old_timeout; \
+ \
+ /* Cancel any existing alarm. */ \
+ old_timeout = alarm (0); \
+ \
+ /* Establish signal handler. */ \
+ 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. */ \
+ memset (&fl, '\0', sizeof (struct flock)); \
+ fl.l_type = (type); \
+ fl.l_whence = SEEK_SET; \
+ fcntl ((fd), F_SETLKW, &fl)
+
+#define UNLOCK_FILE(fd) \
+ /* Unlock the file. */ \
+ fl.l_type = F_UNLCK; \
+ fcntl ((fd), F_SETLKW, &fl); \
+ \
+ /* Reset the signal handler and alarm. */ \
+ sigaction (SIGALRM, &old_action, NULL); \
+ alarm (old_timeout); \
+} while (0)
+
+
/* Functions defined here. */
static int setutent_file (void);
static int getutent_r_file (struct utmp *buffer, struct utmp **result);
@@ -100,7 +143,6 @@
getutent_r_file (struct utmp *buffer, struct utmp **result)
{
ssize_t nbytes;
- struct flock fl; /* Information struct for locking. */
assert (file_fd >= 0);
@@ -111,26 +153,12 @@
return -1;
}
- /* XXX The following is not perfect. Instead of locking the file itself
- Marek Michalkiewicz <marekm@i17linuxb.ists.pwr.wroc.pl> suggests to
- use an extra locking file. */
- /* XXX I think using an extra locking file does not solve the
- problems. Instead we should set an alarm, which causes fcntl to
- fail, as in ../nis/lckcache.c.
- Mark Kettenis <kettenis@phys.uva.nl>. */
-
- /* Try to get the lock. */
- memset (&fl, '\0', sizeof (struct flock));
- fl.l_type = F_RDLCK;
- fl.l_whence = SEEK_SET;
- fcntl (file_fd, F_SETLK, &fl);
+ LOCK_FILE (file_fd, F_RDLCK);
/* Read the next entry. */
nbytes = read (file_fd, &last_entry, sizeof (struct utmp));
- /* And unlock the file. */
- fl.l_type = F_UNLCK;
- fcntl (file_fd, F_SETLKW, &fl);
+ UNLOCK_FILE (file_fd);
if (nbytes != sizeof (struct utmp))
{
@@ -180,13 +208,8 @@
internal_getut_r (const struct utmp *id, struct utmp *buffer)
{
int result = -1;
- struct flock fl;
- /* Try to get the lock. */
- memset (&fl, '\0', sizeof (struct flock));
- fl.l_type = F_RDLCK;
- fl.l_whence = SEEK_SET;
- fcntl (file_fd, F_SETLKW, &fl);
+ LOCK_FILE (file_fd, F_RDLCK);
#if _HAVE_UT_TYPE - 0
if (id->ut_type == RUN_LVL || id->ut_type == BOOT_TIME
@@ -237,9 +260,7 @@
result = 0;
unlock_return:
- /* And unlock the file. */
- fl.l_type = F_UNLCK;
- fcntl (file_fd, F_SETLK, &fl);
+ UNLOCK_FILE (file_fd);
return result;
}
@@ -278,8 +299,6 @@
getutline_r_file (const struct utmp *line, struct utmp *buffer,
struct utmp **result)
{
- struct flock fl;
-
assert (file_fd >= 0);
if (file_offset == -1l)
@@ -288,11 +307,7 @@
return -1;
}
- /* Try to get the lock. */
- memset (&fl, '\0', sizeof (struct flock));
- fl.l_type = F_RDLCK;
- fl.l_whence = SEEK_SET;
- fcntl (file_fd, F_SETLKW, &fl);
+ LOCK_FILE (file_fd, F_RDLCK);
while (1)
{
@@ -322,9 +337,7 @@
*result = buffer;
unlock_return:
- /* And unlock the file. */
- fl.l_type = F_UNLCK;
- fcntl (file_fd, F_SETLK, &fl);
+ UNLOCK_FILE (file_fd);
return ((*result == NULL) ? -1 : 0);
}
@@ -333,7 +346,6 @@
static struct utmp *
pututline_file (const struct utmp *data)
{
- struct flock fl; /* Information struct for locking. */
struct utmp buffer;
struct utmp *pbuf;
int found;
@@ -356,11 +368,7 @@
else
found = internal_getut_r (data, &buffer);
- /* Try to lock the file. */
- memset (&fl, '\0', sizeof (struct flock));
- fl.l_type = F_WRLCK;
- fl.l_whence = SEEK_SET;
- fcntl (file_fd, F_SETLK, &fl);
+ LOCK_FILE (file_fd, F_WRLCK);
if (found < 0)
{
@@ -401,9 +409,7 @@
}
unlock_return:
- /* And unlock the file. */
- fl.l_type = F_UNLCK;
- fcntl (file_fd, F_SETLK, &fl);
+ UNLOCK_FILE (file_fd);
return pbuf;
}
@@ -423,7 +429,6 @@
updwtmp_file (const char *file, const struct utmp *utmp)
{
int result = -1;
- struct flock fl;
off_t offset;
int fd;
@@ -432,11 +437,7 @@
if (fd < 0)
return -1;
- /* Try to get the lock. */
- memset (&fl, '\0', sizeof (struct flock));
- fl.l_type = F_WRLCK;
- fl.l_whence = SEEK_SET;
- fcntl (fd, F_SETLK, &fl);
+ LOCK_FILE (fd, F_WRLCK);
/* Remember original size of log file. */
offset = lseek (fd, 0, SEEK_END);
@@ -461,9 +462,7 @@
result = 0;
unlock_return:
- /* And unlock the file. */
- fl.l_type = F_UNLCK;
- fcntl (fd, F_SETLKW, &fl);
+ UNLOCK_FILE (fd);
/* Close WTMP file. */
close (fd);