Re: [PATCH 1/2] fs: add inode helpers for fsuid and fsgid

[ebiederm@xmission.com (Eric W. Biederman)]

James Bottomley <James.Bottomley@HansenPartnership.com> writes:

> Now that we have two different views of filesystem ids (the filesystem
> view and the kernel view), we have a problem in that
> current_fsuid/fsgid() return the kernel view but are sometimes used in
> filesystem code where the filesystem view shoud be used.  This patch
> introduces helpers to produce the filesystem view of current fsuid and
> fsgid.

If I am reading this right what we are seeing is that xfs explicitly
opted out of type safety with predictable results.   Accidentally
confusing kuids and uids, which is potentially security issue.

All of that said where are you getting sb->s_user_ns != &init_user_ns
for an xfs filesystem?  There are quite a few xfs interfaces that are
not ready for that.   xfs has a very wide userspace interface of ioctls
that all needs to be looked at and addressed carefully if there is
anything like this going on.

I think we really need to ask if we should use kuids and kgids for the
xfs internal quota code.  At the end of the day that is going to be
a whole lot less error prone.

> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
>
> diff --git a/include/linux/cred.h b/include/linux/cred.h
> index f0e70a1..18e9c41 100644
> --- a/include/linux/cred.h
> +++ b/include/linux/cred.h
> @@ -399,4 +399,9 @@ do {						\
>  	*(_fsgid) = __cred->fsgid;		\
>  } while(0)
>  
> +/* return the current id in the filesystem view */
> +#define i_fsuid(i) from_kuid((i)->i_sb->s_user_ns, current_fsuid())
> +#define i_fsgid(i) from_kgid((i)->i_sb->s_user_ns, current_fsgid())

Could we please place these helpers in fs.h?
That should allow them to become inline functions and live with the
existing filesystem helpers in there.

My gut says the names disk_fsuid(i) and disk_fsgid(i) would be clearer.

Of course all of this has the challenge of error handling in the case
when current_fsuid or current_fsgid do not map into the current
filesystem.

Eric