linux-ext4 - Re: [PATCH v8 01/11] timekeeping: move multigrain timestamp floor handling into timekeeper

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <753938ef8e46e9f3d9ea7d977537cd8f5a6533b2.camel@kernel.org>
Date: Mon, 30 Sep 2024 15:27:09 -0400
From: Jeff Layton <jlayton@...nel.org>
To: Thomas Gleixner <tglx@...utronix.de>, John Stultz <jstultz@...gle.com>, 
 Stephen Boyd <sboyd@...nel.org>, Alexander Viro <viro@...iv.linux.org.uk>,
 Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, Steven
 Rostedt <rostedt@...dmis.org>, Masami Hiramatsu <mhiramat@...nel.org>,
 Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Jonathan Corbet
 <corbet@....net>, Chandan Babu R <chandan.babu@...cle.com>, "Darrick J.
 Wong" <djwong@...nel.org>, Theodore Ts'o <tytso@....edu>, Andreas Dilger
 <adilger.kernel@...ger.ca>, Chris Mason <clm@...com>, Josef Bacik
 <josef@...icpanda.com>, David Sterba <dsterba@...e.com>,  Hugh Dickins
 <hughd@...gle.com>, Andrew Morton <akpm@...ux-foundation.org>, Chuck Lever
 <chuck.lever@...cle.com>, Vadim Fedorenko <vadim.fedorenko@...ux.dev>
Cc: Randy Dunlap <rdunlap@...radead.org>, linux-kernel@...r.kernel.org, 
 linux-fsdevel@...r.kernel.org, linux-trace-kernel@...r.kernel.org, 
 linux-doc@...r.kernel.org, linux-xfs@...r.kernel.org,
 linux-ext4@...r.kernel.org,  linux-btrfs@...r.kernel.org,
 linux-nfs@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH v8 01/11] timekeeping: move multigrain timestamp floor
 handling into timekeeper

On Mon, 2024-09-30 at 21:13 +0200, Thomas Gleixner wrote:
> On Sat, Sep 14 2024 at 13:07, Jeff Layton wrote:
> 
> > For multigrain timestamps, we must keep track of the latest timestamp
> > that has ever been handed out, and never hand out a coarse time below
> > that value.
> 
> How is that correct when the clock is off by an hour and then set back
> to the correct value? Then you'd get the same stale timestamp for an
> hour unless something invokes ktime_get_real_ts64_mg() which will set
> the "latest" timestamp back to a time before the previous one.
>
> > Add a static singleton atomic64_t into timekeeper.c that we can use to
> > keep track of the latest fine-grained time ever handed out. This is
> > tracked as a monotonic ktime_t value to ensure that it isn't affected by
> > clock jumps.
> 
> That's just wishful thinking.
> 
> ktime_get_real_ts64_mg(ts)
>    ts = Tmono_1 + offset_1;   // TReal_1
>    floor = Tmono_1;
> 
>                                 // newtime < TReal_1                                
>                                 clock_settime(REALTIME, newtime);
>                                    xtime = newtime; // TReal_2
>                                    offset_2 = offset_1 + Treal_2 - TReal(now);
>                                    --> offset_2 < offset_1
> 
> ktime_get_coarse_real_ts64_mg(ts)
>     ts = tk_xtime();       // TReal_2
>     offs = offset_2;
> 
>     if (Tmono_1 + offset_2 > ts)
>        ts = Tmono_1 + offset_2; // Not taken
> 
> So this returns T_Real_2 because
> 
>     offset_2 < offset_1
> 
> and therefore
> 
>     Tmono_1 + offset_2 < TReal_2
> 
> so the returned time will jump backwards vs. TReal_1 as it should
> because that's the actual time, no?
> 
> So if that's the intended behaviour then the changelog is misleading at
> best.
> 
> If the intention is to never return a value < TReal_1 then this does not
> work. You can make it work by using the Realtime timestamp as floor, but
> that'd be more than questionable vs. clock_settime() making the clock go
> backwards.
> 

That is the intended behavior and I'll plan to fix the changelog to
clarify this point:

If someone jumps the realtime clock backward by a large value, then the
realtime timestamp _can_ appear to go backward. This is a problem today
even without this patchset.

If two files get stamped and a realtime clock jump backward happens in
between them, all bets are off as to which one will appear to have been
modified first. I don't think that is something we can reasonably
prevent, since we must stamp files according to the realtime clock.

The main thing I'm trying to prevent is the timestamps being misordered
in the absence of such a clock jump. Without tracking the floor as I am
here, that's a possibility.
-- 
Jeff Layton <jlayton@...nel.org>