linux-kernel - [PATCH 3/3 v3] fs: make generic_fillattr() tail-callable and utilize it in ext2/ext4

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: <20250406235806.1637000-3-mjguzik@gmail.com>
Date: Mon,  7 Apr 2025 01:58:06 +0200
From: Mateusz Guzik <mjguzik@...il.com>
To: brauner@...nel.org
Cc: viro@...iv.linux.org.uk,
	jack@...e.cz,
	linux-kernel@...r.kernel.org,
	linux-fsdevel@...r.kernel.org,
	Mateusz Guzik <mjguzik@...il.com>
Subject: [PATCH 3/3 v3] fs: make generic_fillattr() tail-callable and utilize it in ext2/ext4

Unfortunately the other filesystems I checked make adjustments after
their own call to generic_fillattr() and consequently can't benefit.

This is a nop for unmodified consumers.

Signed-off-by: Mateusz Guzik <mjguzik@...il.com>
---

v3:
- clarify no callers *need* to change to accmodate this patch

v2:
- also patch vfs_getattr_nosec

There are weird slowdowns on fstat, this submission is a byproduct of
trying to straighten out the fast path.

Not benchmarked, but I did confirm the compiler jmps out to the routine
instead of emitting a call which is the right thing to do here.

that said I'm not going to argue, but I like to see this out of the way.

there are nasty things which need to be addressed separately

 fs/ext2/inode.c    |  3 +--
 fs/ext4/inode.c    |  3 +--
 fs/stat.c          | 10 +++++++---
 include/linux/fs.h |  2 +-
 4 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 30f8201c155f..cf1f89922207 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -1629,8 +1629,7 @@ int ext2_getattr(struct mnt_idmap *idmap, const struct path *path,
 			STATX_ATTR_IMMUTABLE |
 			STATX_ATTR_NODUMP);
 
-	generic_fillattr(&nop_mnt_idmap, request_mask, inode, stat);
-	return 0;
+	return generic_fillattr(&nop_mnt_idmap, request_mask, inode, stat);
 }
 
 int ext2_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 1dc09ed5d403..3edd6e60dd9b 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5687,8 +5687,7 @@ int ext4_getattr(struct mnt_idmap *idmap, const struct path *path,
 				  STATX_ATTR_NODUMP |
 				  STATX_ATTR_VERITY);
 
-	generic_fillattr(idmap, request_mask, inode, stat);
-	return 0;
+	return generic_fillattr(idmap, request_mask, inode, stat);
 }
 
 int ext4_file_getattr(struct mnt_idmap *idmap,
diff --git a/fs/stat.c b/fs/stat.c
index b79ddb83914b..46e10af29f4b 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -78,8 +78,12 @@ 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.
+ *
+ * The routine always succeeds (i.e., nobody needs to check its return value).
+ * We make it return 0 so that consumers can tail-call it at the end of their
+ * own getattr function.
  */
-void generic_fillattr(struct mnt_idmap *idmap, u32 request_mask,
+int generic_fillattr(struct mnt_idmap *idmap, u32 request_mask,
 		      struct inode *inode, struct kstat *stat)
 {
 	vfsuid_t vfsuid = i_uid_into_vfsuid(idmap, inode);
@@ -110,6 +114,7 @@ void generic_fillattr(struct mnt_idmap *idmap, u32 request_mask,
 		stat->change_cookie = inode_query_iversion(inode);
 	}
 
+	return 0;
 }
 EXPORT_SYMBOL(generic_fillattr);
 
@@ -209,8 +214,7 @@ int vfs_getattr_nosec(const struct path *path, struct kstat *stat,
 					    request_mask,
 					    query_flags);
 
-	generic_fillattr(idmap, request_mask, inode, stat);
-	return 0;
+	return generic_fillattr(idmap, request_mask, inode, stat);
 }
 EXPORT_SYMBOL(vfs_getattr_nosec);
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 016b0fe1536e..754893d8d2a8 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3471,7 +3471,7 @@ extern int page_symlink(struct inode *inode, const char *symname, int len);
 extern const struct inode_operations page_symlink_inode_operations;
 extern void kfree_link(void *);
 void fill_mg_cmtime(struct kstat *stat, u32 request_mask, struct inode *inode);
-void generic_fillattr(struct mnt_idmap *, u32, struct inode *, struct kstat *);
+int generic_fillattr(struct mnt_idmap *, u32, struct inode *, struct kstat *);
 void generic_fill_statx_attr(struct inode *inode, struct kstat *stat);
 void generic_fill_statx_atomic_writes(struct kstat *stat,
 				      unsigned int unit_min,
-- 
2.43.0