.Repeated `pututxline` calls following `EINTR` or `EAGAIN` error no longer corrupt the `utmp` file
When the `pututxline` function tries to acquire a lock and does not succeed in time, the function returns with `EINTR` or `EAGAIN` error code. Previously in this situation, if `pututxline` was called immediately again and managed to obtain the lock, it did not use an already-allocated matching slot in the `utmp` file, but added another entry instead. As a consequence, these unused entries increased the size of the `utmp` file substantially. This update fixes the issue, and the entries are added to the `utmp` file correctly now.
DescriptionCarlos O'Donell
2019-09-05 15:45:26 UTC
If pututxline() is repeated after it fails with EINTR or EAGAIN, the utmp file may end up containing some stale entries, as shown below.
There are several issues with pututxline() that have been fixed upstream, and we start with the following commit, which has several dependencies.
commit 61d3db428176d9d0822e4e680305fe34285edff2
Author: Florian Weimer <fweimer>
Date: Wed Aug 28 11:59:45 2019 +0200
login: pututxline could fail to overwrite existing entries [BZ #24902]
The internal_getut_r function updates the file_offset variable and
therefore must always update last_entry as well.
Previously, if pututxline could not upgrade the read lock to a
write lock, internal_getut_r would update file_offset only,
without updating last_entry, and a subsequent call would not
overwrite the existing utmpx entry at file_offset, instead
creating a new entry. This has been observed to cause unbounded
file growth in high-load situations.
This commit removes the buffer argument to internal_getut_r and
updates the last_entry variable directly, along with file_offset.
Initially reported and fixed by Ondřej Lysoněk.
Reviewed-by: Gabriel F. T. Gomes <gabrielftg.com>
We also want:
commit 1a7fe2ebe52b3c8bf465d1756e69452d05c1c103
Author: Florian Weimer <fweimer>
Date: Mon Aug 5 15:54:10 2019 +0200
login: Remove utmp backend jump tables [BZ #23518]
There is just one file-based implementation, so this dispatch
mechanism is unnecessary. Instead of the vtable pointer
__libc_utmp_jump_table, use a non-negative file_fd as the indicator
that the backend is initialized.
commit a33b817f13170b5c24263b92e7e09880fe797d7e
Author: Florian Weimer <fweimer>
Date: Tue Aug 13 12:09:32 2019 +0200
login: Assume that _HAVE_UT_* constants are true
Make the GNU version of bits/utmp.h the generic version because
all remaining ports use it (with a sysdeps override for
Linux s390/s390x).
commit 5a3afa9738f3dbbaf8c0a35665318c1af782111b
Author: Florian Weimer <fweimer>
Date: Tue Aug 13 15:53:19 2019 +0200
login: Replace macro-based control flow with function calls in utmp
commit 341da5b4b6253de9a7581a066f33f89cacb44dec
Author: Florian Weimer <fweimer>
Date: Thu Aug 15 10:30:23 2019 +0200
login: Fix updwtmp, updwtmx unlocking
Commit 5a3afa9738f3dbbaf8c0a35665318c1af782111b (login: Replace
macro-based control flow with function calls in utmp) introduced
a regression because after it, __libc_updwtmp attempts to unlock
the wrong file descriptor.
commit 628598be7e1bfaa04f34df71ef6678f2c5103dfd
Author: Florian Weimer <fweimer>
Date: Thu Aug 15 16:09:05 2019 +0200
login: Disarm timer after utmp lock acquisition [BZ #24879]
If the file processing takes a long time for some reason, SIGALRM can
arrive while the file is still being processed. At that point, file
access will fail with EINTR. Disarming the timer after lock
acquisition avoids that. (If there was a previous alarm, it is the
responsibility of the caller to deal with the EINTR error.)
commit 0d5b2917530ccaf8ad312dfbb7bce69d569c23ad
Author: Florian Weimer <fweimer>
Date: Thu Aug 15 16:09:20 2019 +0200
login: Use struct flock64 in utmp [BZ #24880]
Commit 06ab719d30b01da401150068054d3b8ea93dd12f ("Fix Linux fcntl OFD
locks for non-LFS architectures (BZ#20251)") introduced the use of
fcntl64 into the utmp implementation. However, the lock file
structure was not updated to struct flock64 at that point.
commit c2adefbafcdd2519ff43eca6891c77cd7b29ab62
Author: Florian Weimer <fweimer>
Date: Thu Aug 15 16:09:43 2019 +0200
login: Add nonstring attributes to struct utmp, struct utmpx [BZ #24899]
Commit 7532837d7b03b3ca5b9a63d77a5bd81dd23f3d9c ("The
-Wstringop-truncation option new in GCC 8 detects common misuses")
added __attribute_nonstring__ to bits/utmp.h, but it did not update
the parallel bits/utmpx.h header. In struct utmp, the nonstring
attribute for ut_id was missing.
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.
For information on the advisory, and where to find the updated
files, follow the link below.
If the solution does not work for you, open a new bug report.
https://access.redhat.com/errata/RHSA-2020:1828
If pututxline() is repeated after it fails with EINTR or EAGAIN, the utmp file may end up containing some stale entries, as shown below. There are several issues with pututxline() that have been fixed upstream, and we start with the following commit, which has several dependencies. commit 61d3db428176d9d0822e4e680305fe34285edff2 Author: Florian Weimer <fweimer> Date: Wed Aug 28 11:59:45 2019 +0200 login: pututxline could fail to overwrite existing entries [BZ #24902] The internal_getut_r function updates the file_offset variable and therefore must always update last_entry as well. Previously, if pututxline could not upgrade the read lock to a write lock, internal_getut_r would update file_offset only, without updating last_entry, and a subsequent call would not overwrite the existing utmpx entry at file_offset, instead creating a new entry. This has been observed to cause unbounded file growth in high-load situations. This commit removes the buffer argument to internal_getut_r and updates the last_entry variable directly, along with file_offset. Initially reported and fixed by Ondřej Lysoněk. Reviewed-by: Gabriel F. T. Gomes <gabrielftg.com> We also want: commit 1a7fe2ebe52b3c8bf465d1756e69452d05c1c103 Author: Florian Weimer <fweimer> Date: Mon Aug 5 15:54:10 2019 +0200 login: Remove utmp backend jump tables [BZ #23518] There is just one file-based implementation, so this dispatch mechanism is unnecessary. Instead of the vtable pointer __libc_utmp_jump_table, use a non-negative file_fd as the indicator that the backend is initialized. commit a33b817f13170b5c24263b92e7e09880fe797d7e Author: Florian Weimer <fweimer> Date: Tue Aug 13 12:09:32 2019 +0200 login: Assume that _HAVE_UT_* constants are true Make the GNU version of bits/utmp.h the generic version because all remaining ports use it (with a sysdeps override for Linux s390/s390x). commit 5a3afa9738f3dbbaf8c0a35665318c1af782111b Author: Florian Weimer <fweimer> Date: Tue Aug 13 15:53:19 2019 +0200 login: Replace macro-based control flow with function calls in utmp commit 341da5b4b6253de9a7581a066f33f89cacb44dec Author: Florian Weimer <fweimer> Date: Thu Aug 15 10:30:23 2019 +0200 login: Fix updwtmp, updwtmx unlocking Commit 5a3afa9738f3dbbaf8c0a35665318c1af782111b (login: Replace macro-based control flow with function calls in utmp) introduced a regression because after it, __libc_updwtmp attempts to unlock the wrong file descriptor. commit 628598be7e1bfaa04f34df71ef6678f2c5103dfd Author: Florian Weimer <fweimer> Date: Thu Aug 15 16:09:05 2019 +0200 login: Disarm timer after utmp lock acquisition [BZ #24879] If the file processing takes a long time for some reason, SIGALRM can arrive while the file is still being processed. At that point, file access will fail with EINTR. Disarming the timer after lock acquisition avoids that. (If there was a previous alarm, it is the responsibility of the caller to deal with the EINTR error.) commit 0d5b2917530ccaf8ad312dfbb7bce69d569c23ad Author: Florian Weimer <fweimer> Date: Thu Aug 15 16:09:20 2019 +0200 login: Use struct flock64 in utmp [BZ #24880] Commit 06ab719d30b01da401150068054d3b8ea93dd12f ("Fix Linux fcntl OFD locks for non-LFS architectures (BZ#20251)") introduced the use of fcntl64 into the utmp implementation. However, the lock file structure was not updated to struct flock64 at that point. commit c2adefbafcdd2519ff43eca6891c77cd7b29ab62 Author: Florian Weimer <fweimer> Date: Thu Aug 15 16:09:43 2019 +0200 login: Add nonstring attributes to struct utmp, struct utmpx [BZ #24899] Commit 7532837d7b03b3ca5b9a63d77a5bd81dd23f3d9c ("The -Wstringop-truncation option new in GCC 8 detects common misuses") added __attribute_nonstring__ to bits/utmp.h, but it did not update the parallel bits/utmpx.h header. In struct utmp, the nonstring attribute for ut_id was missing.