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

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 <[email protected]>

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 <[email protected]>

								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 <[email protected]>
> 
-- 
Jan Kara <[email protected]>
SUSE Labs, CR