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

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]

Date:	Wed, 13 Feb 2013 10:13:16 -0800
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Dave Chinner <david@...morbit.com>
Cc:	linux-fsdevel@...r.kernel.org,
	Linux Containers <containers@...ts.linux-foundation.org>,
	linux-kernel@...r.kernel.org, "Serge E. Hallyn" <serge@...lyn.com>,
	Ben Myers <bpm@....com>, Alex Elder <elder@...nel.org>
Subject: Re: [PATCH RFC 10/12] userns: Convert xfs to use kuid/kgid/kprojid where appropriate

Joel Becker <jlbec@...lplan.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/