Re: [PATCH v2] libuuid: Fix pre-1970 UUID v1 timestamp wraparound
Thomas Weißschuh <[email protected]> Wed, 17 Dec 2025 00:21:42 +0100
| Newsgroups | org.kernel.vger.util-linux |
|---|---|
| Message-ID | <[email protected]> |
Hi Kiran, please reply in-line to each individual comment instead of top-posting a single reply. It keeps the conversation understandable. On 2025-12-16 15:40:10-0500, Kiran wrote: > Yes I could work on this. I'll change pre-Unix-epoch v1/v6 UUID > timestamps to be treated as an error. uuid_time_v1() and > uuid_time_v6() will detect underflow, return failure, and > __uuid_time() will propagate it by returning -1 and setting tv_sec = > tv_usec = -1, unless you have any objections. Looking at this more closely: __uuid_time() is directly exposed to users of libuuid as uuid_time(). This means that any change of the function's contract would break all external users and should be avoided at all costs. Fortunately it turns out that the timestamps embedded in UUIDs only use 60 bits. This means that the calculation can be performed in an int64_t without any risk of over- or underflow. Please try to implement it that way instead. It might be useful to change the signature of gregorian_to_unix() to "static void gregorian_to_unix(uint64_t ts, struct timeval *tv)". Also please perform each logical step in a dedicated commit. Thomas > On Tue, Dec 16, 2025 at 8:08 AM Thomas Weißschuh <[email protected]> wrote: > > > > On 2025-12-13 20:01:08-0500, Kiran Rangoon wrote: > > > gregorian_to_unix now returns -1 and sets errno=EOVERFLOW > > > for timestamps before the Unix epoch. uuid_time_v1 and uuid_time_v6 > > > now use signed arithmetic to prevent unsigned wraparound. > > > > > > This fixes uuidparse displaying far-future dates for historical UUIDs. > > > > > > The regression test has been updated to show actual result instead of hardcoded wrong > > > date. > > > > Thanks! > > > > > > > > Example output: > > > $ ./build/uuidparse bf2eb110-d788-1003-aa59-ce1e9e293641 > > > Before: > > > 60041-08-13 16:41:36,271592-04:00 > > > After: > > > 1969-12-31 19:00:00,000000-05:00 > > > > > > Handling negative timestamps gracefully would require broader changes, > > > so I’ve kept this patch focused on preventing pre-1970 wraparound. > > > > IMO we should to this properly. Is this something you want to work on? > > > > > Signed-off-by: Kiran Rangoon <[email protected]> > > > --- > > > libuuid/src/uuid_time.c | 15 +++++++++++---- > > > tests/expected/uuid/uuidparse | 2 +- > > > 2 files changed, 12 insertions(+), 5 deletions(-) > > > > > > diff --git a/libuuid/src/uuid_time.c b/libuuid/src/uuid_time.c > > > index c7516152b..f0d2c8f36 100644 > > > --- a/libuuid/src/uuid_time.c > > > +++ b/libuuid/src/uuid_time.c > > > @@ -60,15 +60,22 @@ > > > /* prototype to make compiler happy */ > > > time_t __uuid_time(const uuid_t uu, struct timeval *ret_tv); > > > > > > -static uint64_t gregorian_to_unix(uint64_t ts) > > > +static int64_t gregorian_to_unix(uint64_t ts) > > > { > > > - return ts - ((((uint64_t) 0x01B21DD2) << 32) + 0x13814000); > > > + const uint64_t offset = 0x01B21DD213814000ULL; > > > + > > > + if (ts < offset) { > > > + errno = EOVERFLOW; > > > + return -1; > > > > The callers do not check for errors. Instead this gets converted to the > > epoch only by chance. > > > > > + } > > > + > > > + return ts - offset; > > > } > > > > > > static void uuid_time_v1(const struct uuid *uuid, struct timeval *tv) > > > { > > > uint32_t high; > > > - uint64_t clock_reg; > > > + int64_t clock_reg; > > > > > > high = uuid->time_mid | ((uuid->time_hi_and_version & 0xFFF) << 16); > > > clock_reg = uuid->time_low | ((uint64_t) high << 32); > > > @@ -80,7 +87,7 @@ static void uuid_time_v1(const struct uuid *uuid, struct timeval *tv) > > > > > > static void uuid_time_v6(const struct uuid *uuid, struct timeval *tv) > > > { > > > - uint64_t clock_reg; > > > + int64_t clock_reg; > > > > > > clock_reg = uuid->time_low; > > > clock_reg <<= 16; > > > diff --git a/tests/expected/uuid/uuidparse b/tests/expected/uuid/uuidparse > > > index 9edb05e4e..0f521a760 100644 > > > --- a/tests/expected/uuid/uuidparse > > > +++ b/tests/expected/uuid/uuidparse > > > @@ -11,7 +11,7 @@ UUID VARIANT TYPE TIME > > > 00000000-0000-3000-8000-000000000000 DCE name-based > > > 00000000-0000-4000-8000-000000000000 DCE random > > > 00000000-0000-5000-8000-000000000000 DCE sha1-based > > > -00000000-0000-6000-8000-000000000000 DCE time-v6 60038-03-11 05:36:10,955161+00:00 > > > +00000000-0000-6000-8000-000000000000 DCE time-v6 1970-01-01 00:00:00,000000+00:00 > > > > If it is an error, the output should be empty. > > > > > 00000000-0000-0000-d000-000000000000 Microsoft > > > 00000000-0000-1000-d000-000000000000 Microsoft > > > 00000000-0000-2000-d000-000000000000 Microsoft > > > -- > > > 2.47.3 >