Bug 1734791

Created attachment 1595073 [details]
reproducer.c

Description of problem:
If pututxline() is repeated after it fails with EINTR or EAGAIN, the utmp file may end up containing some stale entries, as shown below.

Version-Release number of selected component (if applicable):
glibc-2.17-292.el7

How reproducible:
Consistently in my testing. But it's a race condition, so keep trying ;)

Steps to Reproduce:
1. gcc reproducer.c
2. ./a.out
3. last -f /var/run/utmp

Actual results:
A lot of lines like the following:
test     foo:9969     localhost        Tue Jul 30 12:35 - 19:00 (-18107+-16:
test     foo:10561    localhost        Tue Jul 30 12:36 - 19:00 (-18107+-16:
test     foo:8045     localhost        Tue Jul 30 12:35 - 19:00 (-18107+-16:
test     foo:9341     localhost        Tue Jul 30 12:35 - 19:00 (-18107+-16:
test     foo:6528     localhost        Tue Jul 30 12:35 - 19:00 (-18107+-16:

Expected results:
Only a couple of entries for the currently logged in users.

Additional info:
You might suggest calling setutxent() before repeating
pututxline(). This will not work for us in vsftpd for reasons
described in https://bugzilla.redhat.com/show_bug.cgi?id=1688848#c13.
Also, AFAIK, the customary semantics of EINTR/EAGAIN are that the call
in question can be safely repeated without doing any extra work, if
such an error occurs.

The cause of the bug is something along these lines: when pututxline()
is called the first time, the file offset is 0, the internal_getut_r()
function gets called, the file locking therein succeeds and the
function finds the appropriate place to insert the
entry. After it finishes, the file offset is positioned after the last
read entry. Subsequently, the locking directly in pututxline() fails and
pututxline() returns with EINTR. When the pututxline() is repeated, the
file is not rewinded (which is a good thing, though!) and the function
begins searching for the appropriate place for inserting the entry
from the current file offset. It does not find it, because it was
positioned past the entry from the beginning. Consequently, it appends
the entry, even if that is not appropriate.

The attached (naive, proof-of-concept) patch appears to fix the
bug. The idea is to recognize that the previous call to pututxline()
already found the appropriate place to insert the entry and not look
for it again. It does this so that the first locking in
internal_getut_r() can be skipped - this is needed to properly fix
vsftpd bug#1688848.