linux-kernel - Re: [PATCH 01/10] fs: turn inode ctime fields into a single ktime

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: <a11d84a3085c6a6920d086bf8fae1625ceff5764.camel@kernel.org>
Date: Tue, 02 Jul 2024 08:21:42 -0400
From: Jeff Layton <jlayton@...nel.org>
To: Christoph Hellwig <hch@...radead.org>
Cc: Jan Kara <jack@...e.cz>, "Darrick J. Wong" <djwong@...nel.org>, 
 Alexander Viro <viro@...iv.linux.org.uk>, Christian Brauner
 <brauner@...nel.org>, Steven Rostedt <rostedt@...dmis.org>, Masami
 Hiramatsu <mhiramat@...nel.org>, Mathieu Desnoyers
 <mathieu.desnoyers@...icios.com>, Chandan Babu R <chandan.babu@...cle.com>,
 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>, kernel-team@...com, 
 linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, 
 linux-trace-kernel@...r.kernel.org, linux-xfs@...r.kernel.org, 
 linux-ext4@...r.kernel.org, linux-btrfs@...r.kernel.org,
 linux-mm@...ck.org,  linux-nfs@...r.kernel.org
Subject: Re: [PATCH 01/10] fs: turn inode ctime fields into a single ktime_t

On Tue, 2024-07-02 at 05:15 -0700, Christoph Hellwig wrote:
> On Tue, Jul 02, 2024 at 08:09:46AM -0400, Jeff Layton wrote:
> > > > corrupt timestamps like this?
> > > 
> > > inode_set_ctime_to_ts should return an error if things are out of
> > > range.
> > 
> > Currently it just returns the timespec64 we're setting it to (which
> > makes it easy to do several assignments), so we'd need to change
> > its
> > prototype to handle this case, and fix up the callers to recognize
> > the
> > error.
> > 
> > Alternately it may be easier to just add in a test for when
> > __i_ctime == KTIME_MAX in the appropriate callers and have them
> > error
> > out. I'll have a look and see what makes sense.
> 
> The seems like a more awkward interface vs one that explicitly
> checks.
> 

Many of the existing callers of inode_ctime_to_ts are in void return
functions. They're just copying data from an internal representation to
struct inode and assume it always succeeds. For those we'll probably
have to catch bad ctime values earlier.

So, I think I'll probably have to roll bespoke error handling in all of
the relevant filesystems if we go this route. There are also
differences between filesystems -- does it make sense to refuse to load
an inode with a bogus ctime on NFS or AFS? Probably not.

Hell, it may be simpler to just ditch this patch and reimplement
mgtimes using the nanosecond fields like the earlier versions did.

I'll need to study this a bit and figure out what's best.

> > 
> > > How do we currently catch this when it comes from userland?
> > > 
> > 
> > Not sure I understand this question. ctime values should never come
> > from userland. They should only ever come from the system clock.
> 
> Ah, yes, utimes only changes mtime.

Yep. That's the main reason I see the ctime as very different from the
atime or mtime.
-- 
Jeff Layton <jlayton@...nel.org>