Re: [PATCH RFC 10/12] userns: Convert xfs to use kuid/kgid/kprojid where appropriate

Eric W. Biederman
2013-02-13T18:13:29Z

Joel Becker <jlbec@evilplan.org> writes:

> On Wed, Nov 21, 2012 at 10:55:24AM +1100, Dave Chinner wrote:
>> > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
>> > index 2778258..3656b88 100644
>> > --- a/fs/xfs/xfs_inode.c
>> > +++ b/fs/xfs/xfs_inode.c
>> > @@ -570,11 +570,12 @@ xfs_dinode_from_disk(
>> > to->di_version = from ->di_version;
>> > to->di_format = from->di_format;
>> > to->di_onlink = be16_to_cpu(from->di_onlink);
>> > - to->di_uid = be32_to_cpu(from->di_uid);
>> > - to->di_gid = be32_to_cpu(from->di_gid);
>> > + to->di_uid = make_kuid(&init_user_ns, be32_to_cpu(from->di_uid));
>> > + to->di_gid = make_kgid(&init_user_ns, be32_to_cpu(from->di_gid));
>>
>> You can't do this, because the incore inode structure is written
>> directly to the log. This is effectively an on-disk format change.
>
> Yeah, I don't get this either. Over in ocfs2, you do the
> correct thing, translating at the boundary from ocfs2_dinode to struct
> inode.

This is the boundary. The crazy thing is that is that xfs appears to
directly write their incore inode structure into their journal. I had
missed the journal reference the first time through and simply assumed
since this is where the disk inode to the incore inode coversion
happened that the weird scary comment in the xfs header file was wrong.

I would have no problems with not converting those uids and gids except
they are used directly in comparisons against current_fsuid so it really
isn't ok to not convert those values.

I wish that incore structure was managed more like the lvb in ocfs2.
Even though it is cached in core it is first converted into a specific
byte order and then shoved into a character array. Keeping anyone from
getting any funny ideas about using those values directly.

I have sent my xfs changes back to the drawing board and am
concentrating on getting my conversions for the rest of the filesystems
merged. When I get the time I will come back to xfs. xfs is a very
peculiar filesystem.

Eric