mismatch of type of ut_tv.tv_sec between glibc-2.41 and utmp(5)

DISCUSSION Dr. Thomas Orgis 3 participants 5 messages Feb 28, 2025 – Mar 3, 2025 Man-pages: manual pages for Linux -- Sections 2, 3, 4, 5, and 7

lore

Share & tools

lore

Dr. Thomas Orgis

author

2025-02-28 09:45 UTC #0 lore raw

Dear man-pages,

while investigating some old bad usage of time(&ut,ut_time) I noticed that my glibc-2.41 headers define that part of the utmp struct like this:

#if __WORDSIZE_TIME64_COMPAT32
int32_t ut_session; /* Session ID, used for windowing. */
struct
{

    __uint32_t tv_sec;          /* Seconds.  */
    int32_t tv_usec;            /* Microseconds.  */
  } ut_tv;                      /* Time entry was made.  */

#else
long int ut_session; /* Session ID, used for windowing. */
struct timeval ut_tv; /* Time entry was made. */
#endif

The man page claims this:

#if __WORDSIZE == 64 && defined __WORDSIZE_COMPAT32
int32_t ut_session; /* Session ID (getsid(2)),
used for windowing */
struct {
int32_t tv_sec; /* Seconds */
int32_t tv_usec; /* Microseconds */
} ut_tv; /* Time entry was made */
#else
long ut_session; /* Session ID */
struct timeval ut_tv; /* Time entry was made */
#endif

I don't know the history … did it use to be a signed integer and someone decided to buy some time by making it unsigned? This is a minor detail for the bad time() usage, where 32 bit vs. 64 bit time_t might be more serious. Also the macros being checked for this compatibility mode differ, but I am not sure how closely the man page want to follow glibc here.

At least the type of tv_sec should match, I guess.

Now I have to think how elaborately I want to handle possible overflow from time_t assigning to uint32_t with the recommended way of using gettimeofday() for utmp …

Regards,

Thomas

Signature · 2 lines

Dr. Thomas Orgis
HPC @ Universität Hamburg

Florian Weimer

Reviewed-by Adhemerval Zanella

2025-02-28 12:18 UTC #1 lore raw

* Thomas Orgis:

Quoted 2 lines

I don't know the history … did it use to be a signed integer and someone decided to buy some time by making it unsigned?

Yes, this changed in glibc 2.40. Some distributions may have backported it.

commit 5361ad3910c257bc327567be76fde532ed238e42

Author: Florian Weimer <[email protected]>
Date:   Fri Apr 19 14:38:17 2024 +0200

These fields store timestamps when the system was running. No Linux systems existed before 1970, so these values are unused. Switching to unsigned types allows continued use of the existing struct layouts beyond the year 2038.

The intent is to give distributions more time to switch to improved interfaces that also avoid locking/data corruption issues.

Reviewed-by Adhemerval Zanella <[email protected]>

Quoted 3 lines

Now I have to think how elaborately I want to handle possible overflow from time_t assigning to uint32_t with the recommended way of using gettimeofday() for utmp …

On the other hand, utmp is pretty much obsolete because the design assumes a fixed number of terminal lines.

Alejandro Colomar

maintainer Reviewed-by Adhemerval Zanella

2025-02-28 14:06 UTC #2 lore raw

In reply to Dr. Thomas Orgis

Hi Thomas,

Quoted 19 lines · Dr. Thomas Orgis

Dear man-pages,
while investigating some old bad usage of time(&ut,ut_time) I noticed that my glibc-2.41 headers define that part of the utmp struct like this:
#if __WORDSIZE_TIME64_COMPAT32
int32_t ut_session; /* Session ID, used for windowing. */
struct
{
    __uint32_t tv_sec;          /* Seconds.  */
    int32_t tv_usec;            /* Microseconds.  */
  } ut_tv;                      /* Time entry was made.  */
#else
long int ut_session; /* Session ID, used for windowing. */
struct timeval ut_tv; /* Time entry was made. */
#endif
The man page claims this:

Quoted 11 lines

#if __WORDSIZE == 64 && defined __WORDSIZE_COMPAT32
int32_t ut_session; /* Session ID (getsid(2)),
used for windowing */
struct {
int32_t tv_sec; /* Seconds */
int32_t tv_usec; /* Microseconds */
} ut_tv; /* Time entry was made */
#else
long ut_session; /* Session ID */
struct timeval ut_tv; /* Time entry was made */
#endif

It seems your suspicion was right. Someone decided to borrow some time, according to the commit message that changed that code in glibc:

	alx@debian:~/src/gnu/glibc/master$ grep -rn '__uint32_t tv_sec;' -l \
		| xargs -L1 git blame -- \
		| grep '__uint32_t tv_sec;';
	5361ad3910c bits/utmp.h                (Florian Weimer 2024-04-19 14:38:17 +0200  79)     __uint32_t tv_sec;		/* Seconds.  */
	5361ad3910c (Florian Weimer 2024-04-19 14:38:17 +0200  77)     __uint32_t tv_sec;		/* Seconds.  */
	alx@debian:~/src/gnu/glibc/master$ git log -1 5361ad3910c --stat;
	commit 5361ad3910c257bc327567be76fde532ed238e42
	Author: Florian Weimer <[email protected]>
	Date:   Fri Apr 19 14:38:17 2024 +0200

The intent is to give distributions more time to switch to improved interfaces that also avoid locking/data corruption issues.

Reviewed-by Adhemerval Zanella <[email protected]>

Quoted 7 lines

I don't know the history … did it use to be a signed integer and someone decided to buy some time by making it unsigned? This is a minor detail for the bad time() usage, where 32 bit vs. 64 bit time_t might be more serious. Also the macros being checked for this compatibility mode differ, but I am not sure how closely the man page want to follow glibc here.

We should document the change. If anyone wants to send a patch, I'll review it. I won't write it myself, because I'm not an expert in compatibility code between 32 and 64 bits, so I prefer if someone more expert makes sure the documentation is correct.

Quoted 1 line

At least the type of tv_sec should match, I guess.

Yes. And we should probably document the old type in the HISTORY section.

Quoted 3 lines

Now I have to think how elaborately I want to handle possible overflow from time_t assigning to uint32_t with the recommended way of using gettimeofday() for utmp …

If you show some code, we can have a look at it. :)

Have a lovely day!
Alex

Signature · 1 line

<https://www.alejandro-colomar.es/>

Dr. Thomas Orgis

author

2025-03-03 09:46 UTC #3 lore raw

Am Fri, 28 Feb 2025 15:06:33 +0100
schrieb Alejandro Colomar <[email protected]>:

Quoted 7 lines

It seems your suspicion was right. Someone decided to borrow some time, according to the commit message that changed that code in glibc:
We should document the change. If anyone wants to send a patch, I'll review it. I won't write it myself, because I'm not an expert in compatibility code between 32 and 64 bits, so I prefer if someone more expert makes sure the documentation is correct.

Florian? Seems to be the best qualified. As a user, a question for me is if the specific macros deciding for the 32 bit field should be in the man page or not, as they seem to be an implementation detail that one cannot rely on. I guess I need to do a check like sizeof(time_t) > sizeof(tv_sec)? And then just assume unsigned type? It is a hack, I understand. So any use will be hacky.

Quoted 1 line

If you show some code, we can have a look at it. :)

Not much to show. It is semi-abandoned code in simpleinit-msb that called time() directly on tv_sec. I switched that to gettimeofday() and kept the same level of error handling … which is none. My function detects if the assignment to tv_sec changed the time_t value and returns an error, but the caller code is inside void functions and doesn't have an error handling path.

The effect will be broken lastlog entries in a few decades. The unsigned int extension moves that a bit further back.

Alrighty then,

Thomas

Signature · 2 lines

Dr. Thomas Orgis
HPC @ Universität Hamburg

Alejandro Colomar

maintainer

2025-03-03 09:47 UTC #4 lore raw

Hi Thomas,

Quoted 11 lines · Dr. Thomas Orgis

Am Fri, 28 Feb 2025 15:06:33 +0100
schrieb Alejandro Colomar <[email protected]>:
Quoted 7 lines
It seems your suspicion was right. Someone decided to borrow some time, according to the commit message that changed that code in glibc:
We should document the change. If anyone wants to send a patch, I'll review it. I won't write it myself, because I'm not an expert in compatibility code between 32 and 64 bits, so I prefer if someone more expert makes sure the documentation is correct.

Quoted 6 lines

Florian? Seems to be the best qualified. As a user, a question for me is if the specific macros deciding for the 32 bit field should be in the man page or not, as they seem to be an implementation detail that one cannot rely on. I guess I need to do a check like sizeof(time_t) > sizeof(tv_sec)? And then just assume unsigned type? It is a hack, I understand. So any use will be hacky.

Hmmm, this reminds me of timespec(3type). Maybe we can document it similarly here. Feel free to inspire on that page for a patch (you or Florian).

Quoted 11 lines

Quoted 1 line
If you show some code, we can have a look at it. :)
Not much to show. It is semi-abandoned code in simpleinit-msb that called time() directly on tv_sec. I switched that to gettimeofday() and kept the same level of error handling … which is none. My function detects if the assignment to tv_sec changed the time_t value and returns an error, but the caller code is inside void functions and doesn't have an error handling path.
The effect will be broken lastlog entries in a few decades. The unsigned int extension moves that a bit further back.

Thanks for the details!

Have a lovely day!
Alex

Signature · 1 line

<https://www.alejandro-colomar.es/>