Implement ability to use both utmp and logind #1284

Karlson2k · 2025-07-10T19:55:42Z

This PR adds ability to use BOTH utmp and logind.
If both enabled, then for the number of active sessions logind used first, and utmp is used as a fallback. The same for "get_session_host": logind tried first and then utmp tried as a fallback.
Other utmp functionality enabled independently from logind.

This patch is needed because modern systemd (at least 256, 257) does not update utmp. Neither pad_systemd, nor logind does not write information to utmp.
This still breaks some software, including basic logname command.

With the patch utmp and logind can be enabled/disabled independently, allowing building a package that suits particular needs.
The only limitation is that utmp and logind cannot be simultaneously disabled (at least one option must be enabled).

The updated test has not been check for proper functioning on my machine.

alejandro-colomar · 2025-07-11T01:15:40Z

lib/limits.c


-	count = active_sessions_count(name, limit);
+#ifdef ENABLE_LOGIND
+	if (exceeded < 0)


Isn't this dead code? The condition is always true, because exceeded is -1, so you could do that unconditionally, right?

alejandro-colomar · 2025-07-11T01:16:47Z

lib/prototypes.h

+extern int active_sessions_limit_exceeded_utmp(const char *name,
+											   unsigned long limit);


There is some alignment mistake here.

alejandro-colomar · 2025-07-11T01:18:09Z

lib/prototypes.h

+extern int active_sessions_limit_exceeded_lgnd(const char *name,
+											   unsigned long limit);


Alignment. Please use spaces for alignment (tabs for indentation).

alejandro-colomar · 2025-07-11T01:18:49Z

lib/prototypes.h

-extern unsigned long active_sessions_count(const char *name,
-                                           unsigned long limit);
+extern int active_sessions_limit_exceeded_lgnd(const char *name,
+											   unsigned long limit);


Since this API set is becoming big and complex, it would be interesting to move it to its own header files, "lib/logind.h" and "lib/utmp.h".

alejandro-colomar · 2025-07-11T01:20:55Z

lib/utmp.c

-	while ((ut = getutxent()))
+	while ((ut = getutxent()) && (count <= limit))


Please don't add redundant parens (the existing ones are for allowing =).

alejandro-colomar · 2025-07-11T01:21:45Z

lib/utmp.c

-		if (count > limit) {
-			break;
-		}


Why is this moved to the while condition? Is it necessary, or is it cosmetic? If the latter, please do it in a separate commit.

alejandro-colomar · 2025-07-11T01:22:11Z

src/login.c


-	err = get_session_host(&host);
+#ifdef ENABLE_LOGIND
+	if (0 != err)


alejandro-colomar · 2025-07-11T01:24:11Z

lib/limits.c

 {
-	unsigned long limit, count;
+	unsigned long limit;
+	int exceeded = -1;


Please, don't initialize. Prefer assignment closer to use. Initialization of variables can hide logic bugs.

alejandro-colomar · 2025-07-11T01:24:48Z

lib/limits.c

-	if (count > limit) {
+	if (exceeded > 0) {


What if it's still -1? Is that possible?

alejandro-colomar · 2025-07-11T01:25:39Z

lib/logind.c

 }

-unsigned long active_sessions_count(const char *name, MAYBE_UNUSED unsigned long limit)
+int active_sessions_limit_exceeded_lgnd(const char *name, unsigned long limit)


exceeds, in present, is probably more conventional, no?

Or I don't know. I'm not convinced by the function name, but I don't want to strongly oppose it if you want it.

English is not my native language, so I'm not very picky in English words. :)
I'm fine with renaming it to active_sessions_limit_exceeds_lgnd.
Probably it should be renamed to active_sessions_limit_ok_lgnd (with reversing the logic).
Select whatever is nice for you and I'll update the code.

Please change it to exceeds for now. I'm not sure about ok.

(I still don't fully understand this, so we may need to come back to this issue. Sorry.)

ikerexxe · 2025-07-11T12:56:17Z

I have significant concerns about this approach. As you've noted, this change could potentially break basic operating system software, and it appears to exacerbate the problem rather than mitigating it.

Currently, the established practice is to choose between utmp or logind for session management. When distributions decide to transition from one to the other, it typically involves a coordinated change in the build configuration. For example, in Fedora, a change of this magnitude would necessitate a Fedora System-Wide Change proposal to ensure proper communication and adaptation across the ecosystem. Other distributions likely have similar mechanisms.

While I'm not privy to the specific reasons behind systemd's shift away from updating utmp directly, I believe that the solution should involve proper communication and adaptation from systemd to other packages, rather than introducing a dual-system approach that could lead to further fragmentation and instability.

Introducing a scenario where both utmp and logind can be simultaneously enabled, with a fallback mechanism, seems to complicate the system and potentially lead to unexpected behavior and maintenance challenges down the line. The current "either/or" model, while perhaps requiring more effort for transitions, ensures a clearer and more predictable session management environment.

alejandro-colomar · 2025-07-11T13:05:02Z

Cc: @thesamesam

Karlson2k · 2025-07-11T14:59:50Z

I have significant concerns about this approach. As you've noted, this change could potentially break basic operating system software, and it appears to exacerbate the problem rather than mitigating it.

Actually, the basic system software is already broken.
This approach allows to choose the way, how things are handled. It does not enforce to use both utmp and logind.

Currently, the established practice is to choose between utmp or logind for session management. When distributions decide to transition from one to the other, it typically involves a coordinated change in the build configuration. For example, in Fedora, a change of this magnitude would necessitate a Fedora System-Wide Change proposal to ensure proper communication and adaptation across the ecosystem. Other distributions likely have similar mechanisms.

And this will continue to work perfectly fine. Fedora still can use logind-only mode.

While I'm not privy to the specific reasons behind systemd's shift away from updating utmp directly, I believe that the solution should involve proper communication and adaptation from systemd to other packages, rather than introducing a dual-system approach that could lead to further fragmentation and instability.

I cannot see any instability, that can be introduced by this change.
For example, instead of silently accepting more logins than allowed by limit (as it currently done when number of logins cannot be acquired from systemd), a fallback path is used.

This PR can be extended with --enable-utmp=update-only mode, which could be enabled only together with systemd. In such mode utmp could be only updated, but not used as a fallback.

However, I'm sure that fallback is better than nothing when systemd fails.

Introducing a scenario where both utmp and logind can be simultaneously enabled, with a fallback mechanism, seems to complicate the system and potentially lead to unexpected behavior and maintenance challenges down the line. The current "either/or" model, while perhaps requiring more effort for transitions, ensures a clearer and more predictable session management environment.

This situation is already broken and must be fixed now.
For example, on Gentoo with current stable systemd (sys-apps/systemd-257.6) console sessions are incorrect: any software relays on logname command or getlogin() function is not working properly (while the same software in terminal emulators under KDE is fine). The output is always literally LOGIN instead of the name of the actual user. (@thesamesam I decided not to open a Gentoo bug as it is a clear upstream problem).
coreutls (for logname command) will not fix this problem:
https://github.com/coreutils/coreutils/blob/8f9fc8f08c10c3b097211f95c6354a85d41f1101/src/logname.c#L72-L74

Glibc uses audit first (disabled on many distributions and has global performance penalty) and then utmp as a fallback:
https://github.com/bminor/glibc/blob/d6c2760ef7f7cdeab912767f04db4b14632fbb5f/sysdeps/unix/sysv/linux/getlogin.c#L34-L38
This is a getlogin_fd0 () implementation:
https://github.com/bminor/glibc/blob/d6c2760ef7f7cdeab912767f04db4b14632fbb5f/sysdeps/unix/getlogin.c

Probably glibc will never be fixed:
https://github.com/thkukuk/utmpx/blob/main/Y2038.md#glibc

In short:

full transition may take years, while the situation is broken already.
systemd is still updating utmp: https://www.freedesktop.org/software/systemd/man/latest/systemd.exec.html#UtmpMode=, but these updates are limited ("user" mode will not work if user is not specified for the service, i.e. login must still update utmp otherwise it stuck with LOGIN entry for "login process").
agetty is still updating utmp: https://github.com/util-linux/util-linux/blob/6c87a3ac5ee96194e0e461db24b1bbfe369bdb59/term-utils/agetty.c#L976
neither logind nor pam_systemd.so updates utmp.

This looks inconsistent (utmp is updated before firing login binary, but not by the login binary, neither during login process) and the best solution is to add build-time option for utmp manipulation, like suggested in this PR.
I'd emphasise: this build options does not enforce utmp updates for these users who do not need it.

When other software will be fixed, or utmp is completely eliminated, or when one does not want utmp updates, support for utmp in shadow can be easily disabled at build-time

Karlson2k · 2025-07-12T18:38:14Z

@thesamesam PR #1282 is needed to complete the fix for Gentoo.
See also details in the comment #1282 (comment) (when built without logind).

alejandro-colomar · 2025-07-12T21:04:42Z

@thesamesam PR #1282 is needed to complete the fix for Gentoo. See also details in the comment #1282 (comment) (when built without logind).

Is there a Gentoo bug ticket? Could you please link to it?

ikerexxe · 2025-07-14T08:38:43Z

Actually, the basic system software is already broken. This approach allows to choose the way, how things are handled. It does not enforce to use both utmp and logind.

I must have misunderstood the PR because I understood that the fallback was going to be forced. Since you are making it configurable I see it as fine.

Probably glibc will never be fixed: https://github.com/thkukuk/utmpx/blob/main/Y2038.md#glibc

From my conversations with their developers it's not going to be fixed, so we will all have to migrate to logind (or some other mechanism) sooner rather than later.

thesamesam · 2025-07-14T08:44:15Z

(Just saying that I've seen the discussion(s) and I will look properly, but I can't look just yet and it's not the kind of thing I can give a quick take on. Thank you for the ping!)

ikerexxe · 2025-07-14T08:43:40Z

lib/logind.c

    count = sd_uid_get_sessions(pw->pw_uid, 0, NULL);

-    return count;
+	return (count > limit) ? 1 : 0;


Indentation issue

ikerexxe · 2025-07-14T08:46:00Z

tests/unit/test_logind.c

 /***********************
 * TEST
 **********************/
 static void test_active_sessions_count_return_ok(void **state)


Since you are changing the meaning of the test I'd also change the name of the function.

ikerexxe · 2025-07-14T08:46:59Z

tests/unit/test_logind.c

+        cmocka_unit_test(\
+        test_active_sessions_limit_exceeded_lgnd_return_exceeded),
+        cmocka_unit_test(\
+        test_active_sessions_limit_exceeded_lgnd_prefix_failure),


Let's keep this in the same line

The line becomes longer then 80 chars.
If it is not a problem, I'll move back to the same line.

In general, I prefer to avoid surpassing the right margin, but in this case, let's keep it in one line. Tests are special, anyway.

Yes, let's keep it in the same line

ikerexxe · 2025-07-14T08:53:53Z

lib/utmp.c

-unsigned long
-active_sessions_count(const char *name, unsigned long limit)
+int
+active_sessions_limit_exceeded_utmp(const char *name, unsigned long limit)


This function doesn't return -1 on error paths like setutxent() or getutxent() failures. Currently, if setutxent() fails, it will just iterate through 0 entries and return 0 (not exceeded). If getutxent() fails to return anything, it also returns 0. This means utmp might silently fail to check the limit and incorrectly report "not exceeded" if there's an issue with utmp file access.

Yes, good point.
I'll update the code.

Karlson2k · 2025-07-15T11:10:42Z

@alejandro-colomar Before further updating this PR, let's finish #1292 first, as this PR should be used on top.

alejandro-colomar · 2025-08-16T22:05:12Z

This needs to be rebased.

alejandro-colomar · 2025-12-11T11:10:34Z

This would need some rebasing.

Karlson2k · 2025-12-27T14:19:15Z

I'll take care about it soon or shortly after the New Year.

Implement ability to use both utmp and logind

07cad26

alejandro-colomar reviewed Jul 11, 2025

View reviewed changes

src/login.c

err = get_session_host(&host);

#ifdef ENABLE_LOGIND

if (0 != err)

Copy link

Collaborator

alejandro-colomar Jul 11, 2025

Choose a reason for hiding this comment

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

Dead code?

alejandro-colomar reviewed Jul 11, 2025

View reviewed changes

ikerexxe reviewed Jul 14, 2025

View reviewed changes

Karlson2k mentioned this pull request Jul 17, 2025

Correct finding UTMP entry #1292

Merged

alejandro-colomar assigned Karlson2k Dec 27, 2025

		extern int active_sessions_limit_exceeded_utmp(const char *name,
		unsigned long limit);

		while ((ut = getutxent()))
		while ((ut = getutxent()) && (count <= limit))

Implement ability to use both utmp and logind #1284

Are you sure you want to change the base?

Implement ability to use both utmp and logind #1284

Uh oh!

Conversation

Karlson2k commented Jul 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alejandro-colomar Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alejandro-colomar Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alejandro-colomar Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ikerexxe commented Jul 11, 2025

Uh oh!

alejandro-colomar commented Jul 11, 2025

Uh oh!

Karlson2k commented Jul 11, 2025

Uh oh!

Karlson2k commented Jul 12, 2025

Uh oh!

alejandro-colomar commented Jul 12, 2025

Uh oh!

ikerexxe commented Jul 14, 2025

Uh oh!

thesamesam commented Jul 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Karlson2k Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alejandro-colomar Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Karlson2k commented Jul 15, 2025

Uh oh!

alejandro-colomar commented Aug 16, 2025

Uh oh!

alejandro-colomar commented Dec 11, 2025

Uh oh!

Karlson2k commented Dec 27, 2025

alejandro-colomar Jul 11, 2025 •

edited

Loading

alejandro-colomar Jul 11, 2025 •

edited

Loading

alejandro-colomar Jul 11, 2025 •

edited

Loading

Karlson2k Jul 15, 2025 •

edited

Loading

alejandro-colomar Jul 15, 2025 •

edited

Loading