linux-kernel - Re: [PATCH] pipe: don't update {a,c,m}time for anonymous pipes

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: <20250204-autohandel-gebastelt-0ca4424b697b@brauner>
Date: Tue, 4 Feb 2025 17:34:48 +0100
From: Christian Brauner <brauner@...nel.org>
To: Mateusz Guzik <mjguzik@...il.com>, Oleg Nesterov <oleg@...hat.com>, 
	Linus Torvalds <torvalds@...ux-foundation.org>
Cc: David Howells <dhowells@...hat.com>, 
	"Gautham R. Shenoy" <gautham.shenoy@....com>, K Prateek Nayak <kprateek.nayak@....com>, 
	Neeraj Upadhyay <Neeraj.Upadhyay@....com>, Oliver Sang <oliver.sang@...el.com>, 
	Swapnil Sapkal <swapnil.sapkal@....com>, WangYuli <wangyuli@...ontech.com>, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] pipe: don't update {a,c,m}time for anonymous pipes

On Tue, Feb 04, 2025 at 03:21:18PM +0100, Mateusz Guzik wrote:
> On Tue, Feb 4, 2025 at 2:22 PM Oleg Nesterov <oleg@...hat.com> wrote:
> > These numbers are visible in fstat() but hopefully nobody uses this
> > information and file_accessed/file_update_time are not that cheap.
> 
> I'll note majority of the problem most likely stems from
> mnt_get_write_access rolling with:

So anonymous pipes are based on pipefs which isn't exposed to userspace
at all. Neither the superblock nor an individual mount can go read-only
or change mount properties. The superblock cannot be frozen etc.

So really that mnt_get_write_access() should be pointless for
anonymous pipes. In other words, couldn't this also just be:

diff --git a/fs/pipe.c b/fs/pipe.c
index 8df42e1ab3a3..e9989621362c 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -604,12 +604,25 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
        kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
        if (wake_next_writer)
                wake_up_interruptible_sync_poll(&pipe->wr_wait, EPOLLOUT | EPOLLWRNORM);
-       if (ret > 0 && sb_start_write_trylock(file_inode(filp)->i_sb)) {
-               int err = file_update_time(filp);
-               if (err)
-                       ret = err;
-               sb_end_write(file_inode(filp)->i_sb);
+       if (ret > 0) {
+               int err;
+
+               if (!is_anon_pipe(file)) {
+                       if (sb_start_write_trylock(file_inode(filp)->i_sb)) {
+                               err = file_update_time(filp);
+                               if (err)
+                                       ret = err;
+
+                               sb_end_write(file_inode(filp)->i_sb);
+                       }
+               } else {
+                       // Anonymous pipes don't need all the mount + sb bruah.
+                       err = inode_needs_update_time(inode);
+                       if (err > 0)
+                               ret = inode_update_time(file_inode(filp), err);
+               }
        }
+
        return ret;
 }

and then have zero regression risk?