linux-kernel - Re: [RFC] metadata updates vs. fetches (was Re: [PATCH v4] fs: Fix data race in inode_set_ctime_to

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: <CAGudoHGyBQMtfgpq3EzaZi+zWBOoTADVro+Gb27DRHFF1iijVA@mail.gmail.com>
Date: Mon, 25 Nov 2024 00:19:44 +0100
From: Mateusz Guzik <mjguzik@...il.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Al Viro <viro@...iv.linux.org.uk>, "Darrick J. Wong" <djwong@...nel.org>, 
	Hao-ran Zheng <zhenghaoran@...a.edu.cn>, brauner@...nel.org, jack@...e.cz, 
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, 
	baijiaju1990@...il.com, 21371365@...a.edu.cn
Subject: Re: [RFC] metadata updates vs. fetches (was Re: [PATCH v4] fs: Fix
 data race in inode_set_ctime_to_ts)

On Mon, Nov 25, 2024 at 12:05 AM Mateusz Guzik <mjguzik@...il.com> wrote:
> Looks like this is a recurring topic?
>
> Until the day comes when someone has way too much time on their hands
> and patches it up (even that may encounter resistance though), I do
> think it would make sense to nicely write it down somewhere so for
> easy reference -- maybe as a comment above getattr and note around
> other places like the timespec helpers to read that.
>

I'll add personally I was concerned with uid:gid pairs vs chown --
after all you can read an incorrect pair after getting unlucky enough.
Again one has to really try to run into it though.

So how about something like this:
diff --git a/fs/stat.c b/fs/stat.c
index 0870e969a8a0..302586d6afae 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -78,6 +78,11 @@ EXPORT_SYMBOL(fill_mg_cmtime);
  * take care to map the inode according to @idmap before filling in the
  * uid and gid filds. On non-idmapped mounts or if permission checking is to be
  * performed on the raw inode simply pass @nop_mnt_idmap.
+ *
+ * DONTFIXME: no effort is put into ensuring a consistent snapshot of the
+ * metadata read below. For example a call racing against parallel setattr()
+ * can end up with a mixture of old and new attributes. This is not considered
+ * enough to warrant fixing.
  */
 void generic_fillattr(struct mnt_idmap *idmap, u32 request_mask,
                      struct inode *inode, struct kstat *stat)

not an actual patch submission, any party is free to take the comment
and tweak in whatever capacity without credit.

What I am after here is preventing more people from spotting the
problem and thinking it is new.

-- 
Mateusz Guzik <mjguzik gmail.com>