diff options
author | Florian Weimer <fweimer@redhat.com> | 2019-08-28 11:59:45 +0200 |
---|---|---|
committer | Florian Weimer <fweimer@redhat.com> | 2019-08-28 11:59:45 +0200 |
commit | 61d3db428176d9d0822e4e680305fe34285edff2 (patch) | |
tree | 22ad4258b0d656bdf5ece29ade65a3e321a9a1b9 /login/utmp_file.c | |
parent | 3a9d025fdd56f595ef9cb142486da4ee1b75281f (diff) | |
download | glibc-61d3db428176d9d0822e4e680305fe34285edff2.tar.gz |
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@linux.ibm.com>
Diffstat (limited to 'login/utmp_file.c')
-rw-r--r-- | login/utmp_file.c | 21 |
1 files changed, 11 insertions, 10 deletions
diff --git a/login/utmp_file.c b/login/utmp_file.c index 94753e0404..2d0548f6fa 100644 --- a/login/utmp_file.c +++ b/login/utmp_file.c @@ -185,9 +185,11 @@ __libc_getutent_r (struct utmp *buffer, struct utmp **result) } +/* Search for *ID, updating last_entry and file_offset. Return 0 on + success and -1 on failure. If the locking operation failed, write + true to *LOCK_FAILED. */ static int -internal_getut_r (const struct utmp *id, struct utmp *buffer, - bool *lock_failed) +internal_getut_r (const struct utmp *id, bool *lock_failed) { int result = -1; @@ -206,7 +208,7 @@ internal_getut_r (const struct utmp *id, struct utmp *buffer, while (1) { /* Read the next entry. */ - if (__read_nocancel (file_fd, buffer, sizeof (struct utmp)) + if (__read_nocancel (file_fd, &last_entry, sizeof (struct utmp)) != sizeof (struct utmp)) { __set_errno (ESRCH); @@ -215,7 +217,7 @@ internal_getut_r (const struct utmp *id, struct utmp *buffer, } file_offset += sizeof (struct utmp); - if (id->ut_type == buffer->ut_type) + if (id->ut_type == last_entry.ut_type) break; } } @@ -227,7 +229,7 @@ internal_getut_r (const struct utmp *id, struct utmp *buffer, while (1) { /* Read the next entry. */ - if (__read_nocancel (file_fd, buffer, sizeof (struct utmp)) + if (__read_nocancel (file_fd, &last_entry, sizeof (struct utmp)) != sizeof (struct utmp)) { __set_errno (ESRCH); @@ -236,7 +238,7 @@ internal_getut_r (const struct utmp *id, struct utmp *buffer, } file_offset += sizeof (struct utmp); - if (__utmp_equal (buffer, id)) + if (__utmp_equal (&last_entry, id)) break; } } @@ -265,7 +267,7 @@ __libc_getutid_r (const struct utmp *id, struct utmp *buffer, /* We don't have to distinguish whether we can lock the file or whether there is no entry. */ bool lock_failed = false; - if (internal_getut_r (id, &last_entry, &lock_failed) < 0) + if (internal_getut_r (id, &lock_failed) < 0) { *result = NULL; return -1; @@ -330,10 +332,9 @@ unlock_return: struct utmp * __libc_pututline (const struct utmp *data) { - if (!maybe_setutent ()) + if (!maybe_setutent () || file_offset == -1l) return NULL; - struct utmp buffer; struct utmp *pbuf; int found; @@ -369,7 +370,7 @@ __libc_pututline (const struct utmp *data) else { bool lock_failed = false; - found = internal_getut_r (data, &buffer, &lock_failed); + found = internal_getut_r (data, &lock_failed); if (__builtin_expect (lock_failed, false)) { |