Skip to content

Fix incorrect user accounting error handling#57875

Merged
atburke merged 1 commit intomasterfrom
atburke/uacc-logout2
Aug 20, 2025
Merged

Fix incorrect user accounting error handling#57875
atburke merged 1 commit intomasterfrom
atburke/uacc-logout2

Conversation

@atburke
Copy link
Copy Markdown
Contributor

@atburke atburke commented Aug 14, 2025

This change fixes a bug where Teleport would incorrectly check for errors from updwtmp(), which doesn't report any (hence errno is unreliable).

Resolves #54606.

Changelog: Fixed failure to close user accounting session

Comment thread lib/srv/uacc/uacc.h
if (errno != 0) {
return status_from_errno();
}
// updwtmp doesn't report errors, per the c standard we can't trust errno here.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per the c standard

Are you sure this is specified in the C standard? That feels odd to me. Have a reference to the standard text?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3220.pdf
Section 7.5.3: "The value of errno may be set to nonzero by a library function call whether or not there is an error, provided the use of errno is not documented in the description of the function in this document."

@atburke
Copy link
Copy Markdown
Contributor Author

atburke commented Aug 19, 2025

friendly ping @zmb3 @rosstimothy

Copy link
Copy Markdown
Contributor

@boxofrad boxofrad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving because it's clear we shouldn't be checking errno, but I'm unsure about how this fixes the accounting issue. Were we previously checking the uacc_* function return value and then not proceeding to write the session close?

@atburke
Copy link
Copy Markdown
Contributor Author

atburke commented Aug 19, 2025

@boxofrad yep, exactly that

@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from hugoShaka August 19, 2025 19:56
@atburke atburke added this pull request to the merge queue Aug 20, 2025
Merged via the queue into master with commit 1dcfb61 Aug 20, 2025
45 checks passed
@atburke atburke deleted the atburke/uacc-logout2 branch August 20, 2025 17:06
@backport-bot-workflows
Copy link
Copy Markdown
Contributor

@atburke See the table below for backport results.

Branch Result
branch/v16 Create PR
branch/v17 Create PR
branch/v18 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

/var/log/wtmp not updated on session exit

4 participants