Re: [PATCH 1/2] implement uid mount option for ext2

[Andreas Dilger <adilger@sun.com>]

On Jul 24, 2009  12:30 +0200, Ludwig Nussel wrote:
> @@ -1353,7 +1356,13 @@ int ext2_write_inode(struct inode *inode, int do_sync)
>  
>  	ext2_get_inode_flags(ei);
>  	raw_inode->i_mode = cpu_to_le16(inode->i_mode);
> +	if (EXT2_SB(sb)->s_uid &&
> +	    inode->i_uid == EXT2_SB(sb)->s_uid) {
> +		raw_inode->i_uid_high = 0;
> +		raw_inode->i_uid_low  = 0;
> +		raw_inode->i_gid_high = 0;
> +		raw_inode->i_gid_low  = 0;

I would suggest to also clear the SUID flag on this inode.  Otherwise,
it opens the risk of creating SUID root files that might be handled
incorrectly.

To be honest, rather than mapping the specified file to uid == 0/gid == 0
it would be more useful (and safe) to allow specifying a mapping from one
UID to another, or have the on-disk UID always be set to/from the specified
UID.  Given that your original problem is for the user having UIDX on
system X and UIDY on system Y, you should just specify the X->Y mapping
explicitly, instead of an implicit X->0 mapping.  Otherwise, if the user
is unable to access root-owned files on either one of system X or Y your
current patch fails.

I would have the option be something like "uid={local_uid}={disk_uid}"
(which hopefully the option parser can handle), or "uid=X:Y" if not.
That way, the on-disk filesystem will remain correct for at least one
of the two systems.  If someone wants to specify disk_uid=0 that is
fine, but it shouldn't be the only option.

PS - please also send a patch for ext4.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.