>Number:         35135
>Category:       bin
>Synopsis:       timespec patches for utmpentry.c
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Sun Nov 26 21:55:00 +0000 2006
>Originator:     David A. Holland <dholland@eecs.harvard.edu>
>Release:        NetBSD 4.99.3 (-20061125)
>Organization:
   Harvard EECS
>Environment:
System: NetBSD weatherwax 4.99.3 NetBSD 4.99.3 (WEATHERWAX) #2: Wed Oct 18 18:55:11 EDT 2006 dholland@weatherwax:/usr/src/sys/arch/i386/compile/WEATHERWAX i386
Architecture: i386
Machine: i386
>Description:

This morning I ran into an odd situation where talkd let me issue a
talk request to a user who had just logged out, after logout, and
repeatedly so until the currently running talkd process timed out,
exited, and was replaced with a new one by inetd.

The symptoms indicate some kind of problem with the utmp entry caching
code, but I can't find one; the only explanation I have is that the
logout code raced with the utmp reload: because the utmpx updating
code uses stdio, updating a utmp entry is not necessarily atomic.

The theory is that part of the entry was written, updating utmpx's
mtime and causing talkd to begin a reload, but the rest wasn't written
until talkd had read past the entry involved in the logout, and all of
this happened within one second so a subsequent check by talkd would
cause it to think its cached utmp data was still current.

Arguably, the utmpx code should be writing entries atomically;
however, the same thing could happen if two users logged out within
one second of each other and talkd looked at utmp in between.

So here's a patch to the utmp monitoring code used by talkd to have it
check the whole timespec instead of just the time in seconds.

>How-To-Repeat:
good luck.

>Fix:

(tested but not extensively so)

Index: usr.bin/who/utmpentry.c
===================================================================
RCS file: /cvsroot/src/usr.bin/who/utmpentry.c,v
retrieving revision 1.10
diff -u -r1.10 utmpentry.c
--- usr.bin/who/utmpentry.c	20 Sep 2006 19:43:33 -0000	1.10
+++ usr.bin/who/utmpentry.c	26 Nov 2006 19:38:35 -0000
@@ -60,11 +60,11 @@
 
 #ifdef SUPPORT_UTMP
 static void getentry(struct utmpentry *, struct utmp *);
-static time_t utmptime = 0;
+static struct timespec utmptime = {0, 0};
 #endif
 #ifdef SUPPORT_UTMPX
 static void getentryx(struct utmpentry *, struct utmpx *);
-static time_t utmpxtime = 0;
+static struct timespec utmpxtime = {0, 0};
 #endif
 #if defined(SUPPORT_UTMPX) || defined(SUPPORT_UTMP)
 static int setup(const char *);
@@ -134,8 +134,8 @@
 			warn("Cannot stat `%s'", sfname);
 			what &= ~1;
 		} else {
-			if (st.st_mtime > utmpxtime)
-			    utmpxtime = st.st_mtime;
+			if (timespeccmp(&st.st_mtimespec, &utmpxtime, >))
+			    utmpxtime = st.st_mtimespec;
 			else
 			    what &= ~1;
 		}
@@ -148,8 +148,8 @@
 			warn("Cannot stat `%s'", sfname);
 			what &= ~2;
 		} else {
-			if (st.st_mtime > utmptime)
-				utmptime = st.st_mtime;
+			if (timespeccmp(&st.st_mtimespec, &utmptime, >))
+				utmptime = st.st_mtimespec;
 			else
 				what &= ~2;
 		}
@@ -163,10 +163,10 @@
 freeutentries(struct utmpentry *ep)
 {
 #ifdef SUPPORT_UTMP
-	utmptime = 0;
+	timespecclear(&utmptime);
 #endif
 #ifdef SUPPORT_UTMPX
-	utmpxtime = 0;
+	timespecclear(&utmpxtime);
 #endif
 	if (ep == ehead) {
 		ehead = NULL;