[PATCH] fs: document semantics of kstat::{uid,gid} fields

Jann Horn posted 1 patch 1 month, 4 weeks ago
include/linux/stat.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] fs: document semantics of kstat::{uid,gid} fields
Posted by Jann Horn 1 month, 4 weeks ago
The uid stored in struct kstat is logically a vfsuid; file systems
initialize it by converting a kuid (filesystem perspective) to a vfsuid
(mount perspective), then use vfsuid_into_kuid(), which essentially just
typecasts from vfsuid to kuid.

For now, just add a comment to note this mismatch between C type and
semantic type.

Below are some notes for anyone who wants to refactor this in the future.

There are probably two options to refactor this away:

1. Change the type of kstat::uid to vfsuid_t, and perform the conversion
   from vfsuid to userspace-uid in the VFS layer. This wouldn't change
   machine code, just be more semantically correct.
2. Change the semantics of kstat::uid to really be a kuid_t, and let the
   VFS layer take care of doing the translation from kuid to vfsuid that is
   currently done in filesystem code (or in generic_fillattr, on behalf of
   the filesystem code).

Option 2 is probably neater since it moves more logic into the generic VFS
layer, and this is something that is expected to work the same way in all
file systems?

The following coccinelle script:
```
virtual context

@@
struct kstat *stat;
@@
* stat->uid

@@
struct kstat *stat;
@@
* stat->gid

@@
struct kstat stat;
@@
* stat.uid

@@
struct kstat stat;
@@
* stat.gid
```
detects 43 field accesses to these uid/gid fields.

Signed-off-by: Jann Horn <jannh@google.com>
---
 include/linux/stat.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/stat.h b/include/linux/stat.h
index e3d00e7bb26d..9c5709132862 100644
--- a/include/linux/stat.h
+++ b/include/linux/stat.h
@@ -41,8 +41,8 @@ struct kstat {
 	u64		ino;
 	dev_t		dev;
 	dev_t		rdev;
-	kuid_t		uid;
-	kgid_t		gid;
+	kuid_t		uid;		/* This is logically a vfsuid_t. */
+	kgid_t		gid;		/* This is logically a vfsgid_t. */
 	loff_t		size;
 	struct timespec64 atime;
 	struct timespec64 mtime;

---
base-commit: 075b74841bd0065a3bda3440873c747938e69b68
change-id: 20260803-vfs-comment-stat-uid-ea9d874f9368

Best regards,
--  
Jann Horn <jannh@google.com>
Re: [PATCH] fs: document semantics of kstat::{uid,gid} fields
Posted by Christian Brauner 1 month, 2 weeks ago
On Mon, 03 Aug 2026 21:46:19 +0200, Jann Horn wrote:
> fs: document semantics of kstat::{uid,gid} fields

Applied to the vfs-7.3.misc branch of the vfs/vfs.git tree.
Patches in the vfs-7.3.misc branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-7.3.misc

[1/1] fs: document semantics of kstat::{uid,gid} fields
      https://git.kernel.org/vfs/vfs/c/5c47ed98a4ae
Re: [PATCH] fs: document semantics of kstat::{uid,gid} fields
Posted by Jan Kara 1 month, 3 weeks ago
On Mon 03-08-26 21:46:19, Jann Horn wrote:
> The uid stored in struct kstat is logically a vfsuid; file systems
> initialize it by converting a kuid (filesystem perspective) to a vfsuid
> (mount perspective), then use vfsuid_into_kuid(), which essentially just
> typecasts from vfsuid to kuid.
> 
> For now, just add a comment to note this mismatch between C type and
> semantic type.
> 
> Below are some notes for anyone who wants to refactor this in the future.
> 
> There are probably two options to refactor this away:
> 
> 1. Change the type of kstat::uid to vfsuid_t, and perform the conversion
>    from vfsuid to userspace-uid in the VFS layer. This wouldn't change
>    machine code, just be more semantically correct.
> 2. Change the semantics of kstat::uid to really be a kuid_t, and let the
>    VFS layer take care of doing the translation from kuid to vfsuid that is
>    currently done in filesystem code (or in generic_fillattr, on behalf of
>    the filesystem code).
> 
> Option 2 is probably neater since it moves more logic into the generic VFS
> layer, and this is something that is expected to work the same way in all
> file systems?
> 
> The following coccinelle script:
> ```
> virtual context
> 
> @@
> struct kstat *stat;
> @@
> * stat->uid
> 
> @@
> struct kstat *stat;
> @@
> * stat->gid
> 
> @@
> struct kstat stat;
> @@
> * stat.uid
> 
> @@
> struct kstat stat;
> @@
> * stat.gid
> ```
> detects 43 field accesses to these uid/gid fields.
> 
> Signed-off-by: Jann Horn <jannh@google.com>

I don't remember the reason why things are like this - Christian will have
to return from vacation for that :). But I agree with your analysis so feel
free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  include/linux/stat.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/stat.h b/include/linux/stat.h
> index e3d00e7bb26d..9c5709132862 100644
> --- a/include/linux/stat.h
> +++ b/include/linux/stat.h
> @@ -41,8 +41,8 @@ struct kstat {
>  	u64		ino;
>  	dev_t		dev;
>  	dev_t		rdev;
> -	kuid_t		uid;
> -	kgid_t		gid;
> +	kuid_t		uid;		/* This is logically a vfsuid_t. */
> +	kgid_t		gid;		/* This is logically a vfsgid_t. */
>  	loff_t		size;
>  	struct timespec64 atime;
>  	struct timespec64 mtime;
> 
> ---
> base-commit: 075b74841bd0065a3bda3440873c747938e69b68
> change-id: 20260803-vfs-comment-stat-uid-ea9d874f9368
> 
> Best regards,
> --  
> Jann Horn <jannh@google.com>
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
Re: [PATCH] fs: document semantics of kstat::{uid,gid} fields
Posted by Christian Brauner 1 month, 3 weeks ago
On Wed, Aug 05, 2026 at 11:39:32AM +0200, Jan Kara wrote:
> On Mon 03-08-26 21:46:19, Jann Horn wrote:
> > The uid stored in struct kstat is logically a vfsuid; file systems
> > initialize it by converting a kuid (filesystem perspective) to a vfsuid
> > (mount perspective), then use vfsuid_into_kuid(), which essentially just
> > typecasts from vfsuid to kuid.
> > 
> > For now, just add a comment to note this mismatch between C type and
> > semantic type.
> > 
> > Below are some notes for anyone who wants to refactor this in the future.
> > 
> > There are probably two options to refactor this away:
> > 
> > 1. Change the type of kstat::uid to vfsuid_t, and perform the conversion
> >    from vfsuid to userspace-uid in the VFS layer. This wouldn't change
> >    machine code, just be more semantically correct.
> > 2. Change the semantics of kstat::uid to really be a kuid_t, and let the
> >    VFS layer take care of doing the translation from kuid to vfsuid that is
> >    currently done in filesystem code (or in generic_fillattr, on behalf of
> >    the filesystem code).
> > 
> > Option 2 is probably neater since it moves more logic into the generic VFS
> > layer, and this is something that is expected to work the same way in all
> > file systems?
> > 
> > The following coccinelle script:
> > ```
> > virtual context
> > 
> > @@
> > struct kstat *stat;
> > @@
> > * stat->uid
> > 
> > @@
> > struct kstat *stat;
> > @@
> > * stat->gid
> > 
> > @@
> > struct kstat stat;
> > @@
> > * stat.uid
> > 
> > @@
> > struct kstat stat;
> > @@
> > * stat.gid
> > ```
> > detects 43 field accesses to these uid/gid fields.
> > 
> > Signed-off-by: Jann Horn <jannh@google.com>
> 
> I don't remember the reason why things are like this - Christian will have
> to return from vacation for that :). But I agree with your analysis so feel
> free to add:

I already mentioned that to Jann somewhere else. The TL;DR is that the
correct way would be to add vfs{g,u}id_t everywhere so that translation
into vfs ownership happens at the disk <-> kernel and kernel <->
userspace boundaries only. But that's a massive patchset that I decided
against because the real win wasn't all that clear to me. I'm not saying
we shouldn't do it. Especially now with LLM help it is way less tedious
work but it'll be a big patch. :)