RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 1734791 - glibc: Repeating pututxline() on EINTR/EAGAIN causes stale utmp entries
Summary: glibc: Repeating pututxline() on EINTR/EAGAIN causes stale utmp entries
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: glibc
Version: 7.7
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: rc
: ---
Assignee: glibc team
QA Contact: qe-baseos-tools-bugs
URL:
Whiteboard:
Depends On:
Blocks: 1737433
TreeView+ depends on / blocked
 
Reported: 2019-07-31 12:53 UTC by Ondřej Lysoněk
Modified: 2023-03-24 15:08 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 1749439 (view as bug list)
Environment:
Last Closed: 2020-01-27 15:02:22 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
reproducer.c (1.56 KB, text/plain)
2019-07-31 12:53 UTC, Ondřej Lysoněk
no flags Details
proof-of-concept patch (733 bytes, patch)
2019-07-31 12:54 UTC, Ondřej Lysoněk
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
Sourceware 24879 0 P2 RESOLVED login: utmp alarm timer can arrive after lock acquisition 2020-06-09 16:26:41 UTC
Sourceware 24902 0 P2 RESOLVED login: Repeating pututxline on EINTR/EAGAIN causes stale utmp entries 2020-06-09 16:26:41 UTC

Description Ondřej Lysoněk 2019-07-31 12:53:23 UTC
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.

Comment 2 Ondřej Lysoněk 2019-07-31 12:54:50 UTC
Created attachment 1595074 [details]
proof-of-concept patch

Comment 5 Florian Weimer 2019-07-31 13:11:46 UTC
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.

Comment 7 Ondřej Lysoněk 2019-07-31 13:44:50 UTC
(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?

Comment 15 Florian Weimer 2019-08-21 13:40:06 UTC
New patch submitted upstream: https://sourceware.org/ml/libc-alpha/2019-08/msg00562.html

Comment 16 Ondřej Lysoněk 2019-08-23 13:53:54 UTC
Thanks, looks good to me. I'm happy to test it if you prepare a test build.

Comment 26 Carlos O'Donell 2020-01-27 15:02:22 UTC
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.


Note You need to log in before you can comment on or make changes to this bug.