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.
Created attachment 1595074 [details] proof-of-concept patch
We need to change our approach to locking anyway, due to this security bug: https://sourceware.org/bugzilla/show_bug.cgi?id=24492 I do not know if it is worthwhile to fix the EINTR/EAGAIN issue separately.
(In reply to Florian Weimer from comment #5) > We need to change our approach to locking anyway, due to this security bug: > > https://sourceware.org/bugzilla/show_bug.cgi?id=24492 > > I do not know if it is worthwhile to fix the EINTR/EAGAIN issue separately. OK. There was another suggestion I had for improving pututxline(). Currently, when pututxline() writes to the utmp file, it locks the whole file for writing. I don't see a reason it should do that; I think locking only the portion of the file where the write will occur should suffice. Locking only a portion of the file could reduce contention and improve performance. I was going to file a bug for this. Should I still do that, given that you'll be reworking pututxline() anyway?
New patch submitted upstream: https://sourceware.org/ml/libc-alpha/2019-08/msg00562.html
Thanks, looks good to me. I'm happy to test it if you prepare a test build.
This issue is fixed upstream, and in RHEL8. Given the current priority for fixing this in RHEL 7 the glibc team has decided that we will not fix this, therefore I'm closing this CLOSED / WONTFIX. If we have another customer request to fix this issue we may reconsider fixing the bug in later releases.