Hi,
When using 'real' file systems on removable storage devices such as
hard disks or usb sticks people quickly face the problem that their
Linux users have different uids on different machines. Therefore one
cannot modify or even read files created on a different machine
without running chown as root or storing everything with mode 777.
Simple file systems such as vfat don't have that problem as they
don't store file ownership information and one can pass the uid
files should belong to as mount option.
The following two patches (for 2.6.31-rc4) therefore implement the
uid mount option for ext2 and ext3 to make them actually useful on
removable media. My implementation just writes uid 0 to disk for
files that are owned by the specified user. In read direction files
with uid 0 appear as being owned by the specified user.
In an ideal world this would probably be implemented as vfs feature
rather than having it in every single file system.
Anyways, AFAICT the method works just fine for ext2. I'm not sure
about the ext3 patch though as ext3 has that ext3_setattr() function
for journaling. I don't know if the uid should better be mangled
there instead.
cu
Ludwig
--
(o_ Ludwig Nussel
//\
V_/_ http://www.suse.de/
SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nuernberg)
Signed-off-by: Ludwig Nussel <[email protected]>
---
fs/ext3/inode.c | 11 ++++++++++-
fs/ext3/super.c | 8 +++++++-
include/linux/ext3_fs_sb.h | 1 +
3 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
index 5f51fed..148a4d3 100644
--- a/fs/ext3/inode.c
+++ b/fs/ext3/inode.c
@@ -2766,6 +2766,9 @@ struct inode *ext3_iget(struct super_block *sb, unsigned long ino)
inode->i_uid |= le16_to_cpu(raw_inode->i_uid_high) << 16;
inode->i_gid |= le16_to_cpu(raw_inode->i_gid_high) << 16;
}
+ if (EXT3_SB(sb)->s_uid && inode->i_uid == 0) {
+ inode->i_uid = EXT3_SB(sb)->s_uid;
+ }
inode->i_nlink = le16_to_cpu(raw_inode->i_links_count);
inode->i_size = le32_to_cpu(raw_inode->i_size);
inode->i_atime.tv_sec = (signed)le32_to_cpu(raw_inode->i_atime);
@@ -2905,7 +2908,13 @@ static int ext3_do_update_inode(handle_t *handle,
ext3_get_inode_flags(ei);
raw_inode->i_mode = cpu_to_le16(inode->i_mode);
- if(!(test_opt(inode->i_sb, NO_UID32))) {
+ if (EXT3_SB(inode->i_sb)->s_uid &&
+ inode->i_uid == EXT3_SB(inode->i_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;
+ } else if(!(test_opt(inode->i_sb, NO_UID32))) {
raw_inode->i_uid_low = cpu_to_le16(low_16_bits(inode->i_uid));
raw_inode->i_gid_low = cpu_to_le16(low_16_bits(inode->i_gid));
/*
diff --git a/fs/ext3/super.c b/fs/ext3/super.c
index 524b349..b610242 100644
--- a/fs/ext3/super.c
+++ b/fs/ext3/super.c
@@ -779,7 +779,7 @@ enum {
Opt_usrjquota, Opt_grpjquota, Opt_offusrjquota, Opt_offgrpjquota,
Opt_jqfmt_vfsold, Opt_jqfmt_vfsv0, Opt_quota, Opt_noquota,
Opt_ignore, Opt_barrier, Opt_err, Opt_resize, Opt_usrquota,
- Opt_grpquota
+ Opt_grpquota, Opt_uid
};
static const match_table_t tokens = {
@@ -832,6 +832,7 @@ static const match_table_t tokens = {
{Opt_usrquota, "usrquota"},
{Opt_barrier, "barrier=%u"},
{Opt_resize, "resize"},
+ {Opt_uid, "uid=%u"},
{Opt_err, NULL},
};
@@ -1183,6 +1184,11 @@ set_qf_format:
case Opt_bh:
clear_opt(sbi->s_mount_opt, NOBH);
break;
+ case Opt_uid:
+ if (match_int(&args[0], &option))
+ return 0;
+ sbi->s_uid = option;
+ break;
default:
printk (KERN_ERR
"EXT3-fs: Unrecognized mount option \"%s\" "
diff --git a/include/linux/ext3_fs_sb.h b/include/linux/ext3_fs_sb.h
index f07f34d..751058a 100644
--- a/include/linux/ext3_fs_sb.h
+++ b/include/linux/ext3_fs_sb.h
@@ -47,6 +47,7 @@ struct ext3_sb_info {
ext3_fsblk_t s_sb_block;
uid_t s_resuid;
gid_t s_resgid;
+ uid_t s_uid; /* map root owned files to this uid */
unsigned short s_mount_state;
unsigned short s_pad;
int s_addr_per_block_bits;
--
1.6.2.1
Signed-off-by: Ludwig Nussel <[email protected]>
---
fs/ext2/inode.c | 11 ++++++++++-
fs/ext2/super.c | 9 ++++++++-
include/linux/ext2_fs_sb.h | 1 +
3 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index e271303..7324f45 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -1239,6 +1239,9 @@ struct inode *ext2_iget (struct super_block *sb, unsigned long ino)
inode->i_uid |= le16_to_cpu(raw_inode->i_uid_high) << 16;
inode->i_gid |= le16_to_cpu(raw_inode->i_gid_high) << 16;
}
+ if (EXT2_SB(sb)->s_uid && inode->i_uid == 0) {
+ inode->i_uid = EXT2_SB(sb)->s_uid;
+ }
inode->i_nlink = le16_to_cpu(raw_inode->i_links_count);
inode->i_size = le32_to_cpu(raw_inode->i_size);
inode->i_atime.tv_sec = (signed)le32_to_cpu(raw_inode->i_atime);
@@ -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 (!(test_opt(sb, NO_UID32))) {
+ 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;
+ } else if (!(test_opt(sb, NO_UID32))) {
raw_inode->i_uid_low = cpu_to_le16(low_16_bits(uid));
raw_inode->i_gid_low = cpu_to_le16(low_16_bits(gid));
/*
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 1a9ffee..b2ce1c4 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -382,7 +382,8 @@ enum {
Opt_err_ro, Opt_nouid32, Opt_nocheck, Opt_debug,
Opt_oldalloc, Opt_orlov, Opt_nobh, Opt_user_xattr, Opt_nouser_xattr,
Opt_acl, Opt_noacl, Opt_xip, Opt_ignore, Opt_err, Opt_quota,
- Opt_usrquota, Opt_grpquota, Opt_reservation, Opt_noreservation
+ Opt_usrquota, Opt_grpquota, Opt_reservation, Opt_noreservation,
+ Opt_uid
};
static const match_table_t tokens = {
@@ -416,6 +417,7 @@ static const match_table_t tokens = {
{Opt_usrquota, "usrquota"},
{Opt_reservation, "reservation"},
{Opt_noreservation, "noreservation"},
+ {Opt_uid, "uid=%u"},
{Opt_err, NULL}
};
@@ -556,6 +558,11 @@ static int parse_options (char * options,
clear_opt(sbi->s_mount_opt, RESERVATION);
printk("reservations OFF\n");
break;
+ case Opt_uid:
+ if (match_int(&args[0], &option))
+ return 0;
+ sbi->s_uid = option;
+ break;
case Opt_ignore:
break;
default:
diff --git a/include/linux/ext2_fs_sb.h b/include/linux/ext2_fs_sb.h
index 1cdb663..a4b0b79 100644
--- a/include/linux/ext2_fs_sb.h
+++ b/include/linux/ext2_fs_sb.h
@@ -88,6 +88,7 @@ struct ext2_sb_info {
unsigned long s_sb_block;
uid_t s_resuid;
gid_t s_resgid;
+ uid_t s_uid; /* map root owned files to this uid */
unsigned short s_mount_state;
unsigned short s_pad;
int s_addr_per_block_bits;
--
1.6.2.1
On Thu, 23 Jul 2009 13:36:29 +0200 Ludwig Nussel wrote:
> Hi,
>
> When using 'real' file systems on removable storage devices such as
> hard disks or usb sticks people quickly face the problem that their
> Linux users have different uids on different machines. Therefore one
> cannot modify or even read files created on a different machine
> without running chown as root or storing everything with mode 777.
> Simple file systems such as vfat don't have that problem as they
> don't store file ownership information and one can pass the uid
> files should belong to as mount option.
>
> The following two patches (for 2.6.31-rc4) therefore implement the
> uid mount option for ext2 and ext3 to make them actually useful on
> removable media. My implementation just writes uid 0 to disk for
> files that are owned by the specified user. In read direction files
> with uid 0 appear as being owned by the specified user.
>
> In an ideal world this would probably be implemented as vfs feature
> rather than having it in every single file system.
>
> Anyways, AFAICT the method works just fine for ext2. I'm not sure
> about the ext3 patch though as ext3 has that ext3_setattr() function
> for journaling. I don't know if the uid should better be mangled
> there instead.
Hi,
Please document the mount options in Documentation/filesystems/ext?.txt .
Thanks.
---
~Randy
LPC 2009, Sept. 23-25, Portland, Oregon
http://linuxplumbersconf.org/2009/
On Thu, 23 Jul 2009 13:36:29 +0200, Ludwig Nussel said:
> The following two patches (for 2.6.31-rc4) therefore implement the
> uid mount option for ext2 and ext3 to make them actually useful on
> removable media. My implementation just writes uid 0 to disk for
> files that are owned by the specified user.
I'm certain this will end up violating the Principle of Least Surprise.
For instance - you have UID 500 on 2 systems. Mount on old system, create a
file - it's owned by 500. Take it to a new system, mount it, watch it get
smashed to 0 because it's owned by "you". Take it back to the old system, and
hey, you can't edit your file because it's not owned by 500 anymore...
Hint: This *same exact* problem has been an issue for NFS for at least 25
years. Might want to think about (a) why Yellow Pages (and later LDAP) was
developed, and (b) why NFS "root squash" traditionally maps to "nobody" rather
than a usable UID.
[email protected] wrote:
> On Thu, 23 Jul 2009 13:36:29 +0200, Ludwig Nussel said:
>
> > The following two patches (for 2.6.31-rc4) therefore implement the
> > uid mount option for ext2 and ext3 to make them actually useful on
> > removable media. My implementation just writes uid 0 to disk for
> > files that are owned by the specified user.
>
> I'm certain this will end up violating the Principle of Least Surprise.
>
> For instance - you have UID 500 on 2 systems. Mount on old system, create a
> file - it's owned by 500. Take it to a new system, mount it, watch it get
> smashed to 0 because it's owned by "you". Take it back to the old system, and
> hey, you can't edit your file because it's not owned by 500 anymore...
Yes. However, to actually enable the mapping you have to explicitly
add uid=useruid in the fstab of the new system. So if you know your
uid is the same everywhere just don't do that. Everything behaves as
it does today then. If you use hal for mounting it depends on the
policy files and the GUI whether or not you are allowed to override
that mount option.
> Hint: This *same exact* problem has been an issue for NFS for at least 25
> years. Might want to think about (a) why Yellow Pages (and later LDAP) was
> developed, and (b) why NFS "root squash" traditionally maps to "nobody" rather
> than a usable UID.
I don't see how that helps us here. Sure, a uid != 0 could be used
for the mapping but which one? The uid of user 'nobody' is not the
same across distros.
cu
Ludwig
--
(o_ Ludwig Nussel
//\
V_/_ http://www.suse.de/
SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nuernberg)
Signed-off-by: Ludwig Nussel <[email protected]>
---
Documentation/filesystems/ext2.txt | 2 ++
fs/ext2/inode.c | 11 ++++++++++-
fs/ext2/super.c | 9 ++++++++-
include/linux/ext2_fs_sb.h | 1 +
4 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/Documentation/filesystems/ext2.txt b/Documentation/filesystems/ext2.txt
index 67639f9..4cd49e4 100644
--- a/Documentation/filesystems/ext2.txt
+++ b/Documentation/filesystems/ext2.txt
@@ -42,6 +42,8 @@ orlov (*) Use the Orlov block allocator.
resuid=n The user ID which may use the reserved blocks.
resgid=n The group ID which may use the reserved blocks.
+uid=n Map root owned files to this uid.
+
sb=n Use alternate superblock at this location.
user_xattr Enable "user." POSIX Extended Attributes
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index e271303..7324f45 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -1239,6 +1239,9 @@ struct inode *ext2_iget (struct super_block *sb, unsigned long ino)
inode->i_uid |= le16_to_cpu(raw_inode->i_uid_high) << 16;
inode->i_gid |= le16_to_cpu(raw_inode->i_gid_high) << 16;
}
+ if (EXT2_SB(sb)->s_uid && inode->i_uid == 0) {
+ inode->i_uid = EXT2_SB(sb)->s_uid;
+ }
inode->i_nlink = le16_to_cpu(raw_inode->i_links_count);
inode->i_size = le32_to_cpu(raw_inode->i_size);
inode->i_atime.tv_sec = (signed)le32_to_cpu(raw_inode->i_atime);
@@ -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 (!(test_opt(sb, NO_UID32))) {
+ 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;
+ } else if (!(test_opt(sb, NO_UID32))) {
raw_inode->i_uid_low = cpu_to_le16(low_16_bits(uid));
raw_inode->i_gid_low = cpu_to_le16(low_16_bits(gid));
/*
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 1a9ffee..b2ce1c4 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -382,7 +382,8 @@ enum {
Opt_err_ro, Opt_nouid32, Opt_nocheck, Opt_debug,
Opt_oldalloc, Opt_orlov, Opt_nobh, Opt_user_xattr, Opt_nouser_xattr,
Opt_acl, Opt_noacl, Opt_xip, Opt_ignore, Opt_err, Opt_quota,
- Opt_usrquota, Opt_grpquota, Opt_reservation, Opt_noreservation
+ Opt_usrquota, Opt_grpquota, Opt_reservation, Opt_noreservation,
+ Opt_uid
};
static const match_table_t tokens = {
@@ -416,6 +417,7 @@ static const match_table_t tokens = {
{Opt_usrquota, "usrquota"},
{Opt_reservation, "reservation"},
{Opt_noreservation, "noreservation"},
+ {Opt_uid, "uid=%u"},
{Opt_err, NULL}
};
@@ -556,6 +558,11 @@ static int parse_options (char * options,
clear_opt(sbi->s_mount_opt, RESERVATION);
printk("reservations OFF\n");
break;
+ case Opt_uid:
+ if (match_int(&args[0], &option))
+ return 0;
+ sbi->s_uid = option;
+ break;
case Opt_ignore:
break;
default:
diff --git a/include/linux/ext2_fs_sb.h b/include/linux/ext2_fs_sb.h
index 1cdb663..a4b0b79 100644
--- a/include/linux/ext2_fs_sb.h
+++ b/include/linux/ext2_fs_sb.h
@@ -88,6 +88,7 @@ struct ext2_sb_info {
unsigned long s_sb_block;
uid_t s_resuid;
gid_t s_resgid;
+ uid_t s_uid; /* map root owned files to this uid */
unsigned short s_mount_state;
unsigned short s_pad;
int s_addr_per_block_bits;
--
1.6.2.1
Signed-off-by: Ludwig Nussel <[email protected]>
---
Documentation/filesystems/ext3.txt | 2 ++
fs/ext3/inode.c | 11 ++++++++++-
fs/ext3/super.c | 8 +++++++-
include/linux/ext3_fs_sb.h | 1 +
4 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/Documentation/filesystems/ext3.txt b/Documentation/filesystems/ext3.txt
index 570f9bd..abdb2f6 100644
--- a/Documentation/filesystems/ext3.txt
+++ b/Documentation/filesystems/ext3.txt
@@ -121,6 +121,8 @@ resgid=n The group ID which may use the reserved blocks.
resuid=n The user ID which may use the reserved blocks.
+uid=n Map root owned files to this uid.
+
sb=n Use alternate superblock at this location.
quota
diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
index 5f51fed..148a4d3 100644
--- a/fs/ext3/inode.c
+++ b/fs/ext3/inode.c
@@ -2766,6 +2766,9 @@ struct inode *ext3_iget(struct super_block *sb, unsigned long ino)
inode->i_uid |= le16_to_cpu(raw_inode->i_uid_high) << 16;
inode->i_gid |= le16_to_cpu(raw_inode->i_gid_high) << 16;
}
+ if (EXT3_SB(sb)->s_uid && inode->i_uid == 0) {
+ inode->i_uid = EXT3_SB(sb)->s_uid;
+ }
inode->i_nlink = le16_to_cpu(raw_inode->i_links_count);
inode->i_size = le32_to_cpu(raw_inode->i_size);
inode->i_atime.tv_sec = (signed)le32_to_cpu(raw_inode->i_atime);
@@ -2905,7 +2908,13 @@ static int ext3_do_update_inode(handle_t *handle,
ext3_get_inode_flags(ei);
raw_inode->i_mode = cpu_to_le16(inode->i_mode);
- if(!(test_opt(inode->i_sb, NO_UID32))) {
+ if (EXT3_SB(inode->i_sb)->s_uid &&
+ inode->i_uid == EXT3_SB(inode->i_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;
+ } else if(!(test_opt(inode->i_sb, NO_UID32))) {
raw_inode->i_uid_low = cpu_to_le16(low_16_bits(inode->i_uid));
raw_inode->i_gid_low = cpu_to_le16(low_16_bits(inode->i_gid));
/*
diff --git a/fs/ext3/super.c b/fs/ext3/super.c
index 524b349..b610242 100644
--- a/fs/ext3/super.c
+++ b/fs/ext3/super.c
@@ -779,7 +779,7 @@ enum {
Opt_usrjquota, Opt_grpjquota, Opt_offusrjquota, Opt_offgrpjquota,
Opt_jqfmt_vfsold, Opt_jqfmt_vfsv0, Opt_quota, Opt_noquota,
Opt_ignore, Opt_barrier, Opt_err, Opt_resize, Opt_usrquota,
- Opt_grpquota
+ Opt_grpquota, Opt_uid
};
static const match_table_t tokens = {
@@ -832,6 +832,7 @@ static const match_table_t tokens = {
{Opt_usrquota, "usrquota"},
{Opt_barrier, "barrier=%u"},
{Opt_resize, "resize"},
+ {Opt_uid, "uid=%u"},
{Opt_err, NULL},
};
@@ -1183,6 +1184,11 @@ set_qf_format:
case Opt_bh:
clear_opt(sbi->s_mount_opt, NOBH);
break;
+ case Opt_uid:
+ if (match_int(&args[0], &option))
+ return 0;
+ sbi->s_uid = option;
+ break;
default:
printk (KERN_ERR
"EXT3-fs: Unrecognized mount option \"%s\" "
diff --git a/include/linux/ext3_fs_sb.h b/include/linux/ext3_fs_sb.h
index f07f34d..751058a 100644
--- a/include/linux/ext3_fs_sb.h
+++ b/include/linux/ext3_fs_sb.h
@@ -47,6 +47,7 @@ struct ext3_sb_info {
ext3_fsblk_t s_sb_block;
uid_t s_resuid;
gid_t s_resgid;
+ uid_t s_uid; /* map root owned files to this uid */
unsigned short s_mount_state;
unsigned short s_pad;
int s_addr_per_block_bits;
--
1.6.2.1
Hi,
this time with Documentation as suggested by Randy Dunlap.
cu
Ludwig
--
(o_ Ludwig Nussel
//\
V_/_ http://www.suse.de/
SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nuernberg)
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.
>>>>> "Andreas" == Andreas Dilger <[email protected]> writes:
Andreas> 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;
Andreas> I would suggest to also clear the SUID flag on this inode.
Andreas> Otherwise, it opens the risk of creating SUID root files that
Andreas> might be handled incorrectly.
Andreas> To be honest, rather than mapping the specified file to uid
Andreas> == 0/gid == 0 it would be more useful (and safe) to allow
Andreas> specifying a mapping from one UID to another, or have the
Andreas> on-disk UID always be set to/from the specified UID. Given
Andreas> that your original problem is for the user having UIDX on
Andreas> system X and UIDY on system Y, you should just specify the
Andreas> X->Y mapping explicitly, instead of an implicit X->0 mapping.
Andreas> Otherwise, if the user is unable to access root-owned files
Andreas> on either one of system X or Y your current patch fails.
I didn't read the original email closely, but I have to say that both
of these plans don't sound good to me. If you can mount a filesystem,
you're root already, so you can do any fixup you need.
If you're sharing the filesystem via a Network Filesystem, then you
have to have proper UIDs that match.
If you're moving a disks (USB, eSATA, whatever) between systems, then
I don't think *either* system should do anything automatically. It's
too fraught with danger.
Andreas> I would have the option be something like
Andreas> "uid={local_uid}={disk_uid}" (which hopefully the option
Andreas> parser can handle), or "uid=X:Y" if not. That way, the
Andreas> on-disk filesystem will remain correct for at least one of
Andreas> the two systems. If someone wants to specify disk_uid=0 that
Andreas> is fine, but it shouldn't be the only option.
So what happens when you have 1000 UIDs on a disk you want to re-map?
How does that happen? Or if this option is just for a single UID
mapping, then it's seems to be just as easy to have a script you run
after mounting to fixup the disk.
Or better yet, just match the UIDs, though I understand why this would
be a problem if you don't control one of the systems completely, or if
you're trying to move the disk between three or more systems and you
can't control two or more of them with conflicting UIDs.
But in that case, you're screwed anyway and it's going to become
un-manageable. Push this to userspace, not the kernel since it's a
userspace issue when you come right down to it.
John
John Stoffel wrote:
> I didn't read the original email closely, but I have to say that both
> of these plans don't sound good to me. If you can mount a filesystem,
> you're root already, so you can do any fixup you need.
What if someone lends you a 1TB disk, for you to browse it in your
favourite GUI or Shell Window to read some files from it? And you're
to put a couple of files on it before you give it back?
Hotplug scripts run as root to mount it, and you have your GUI / Shell
Window which don't run as root to read and write a few of those files.
You must not chown anything on the disk, because it isn't your disk.
> But in that case, you're screwed anyway and it's going to become
> un-manageable. Push this to userspace, not the kernel since it's a
> userspace issue when you come right down to it.
How do you handle the above scenario in userspace?
-- Jamie
Andreas Dilger wrote:
> 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.
Doesn't really matter. The mount options nosuid,nodev are standard
for user mounts ever since people started using floppy disks :-)
> 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.
That's unnecessarily complicated. You don't have to keep track of
your user ids when using e.g. FAT formatted USB memory sticks
either. The files just always magically appear to be owned by the
user who mounted the file system. The goal is to have it just as
simple with ext2 on the USB stick. If one of the systems doesn't
mount media with the uid option the files might be unaccesible,
that's true. IOW on that system the situation is no different from
today any you'll have to resort to the same workarounds you have to
use today already (like sudo chown -R $USER or chmod 777).
> PS - please also send a patch for ext4.
One thing after another :-)
cu
Ludwig
--
(o_ Ludwig Nussel
//\ SUSE LINUX Products GmbH, Development
V_/_ http://www.suse.de/
>>>>> "Jamie" == Jamie Lokier <[email protected]> writes:
Sorry for the delay, I was out of it this weekend... :]
Jamie> John Stoffel wrote:
>> I didn't read the original email closely, but I have to say that both
>> of these plans don't sound good to me. If you can mount a filesystem,
>> you're root already, so you can do any fixup you need.
Jamie> What if someone lends you a 1TB disk, for you to browse it in
Jamie> your favourite GUI or Shell Window to read some files from it?
Jamie> And you're to put a couple of files on it before you give it
Jamie> back?
So? How is the kernel supposed to know you're doing this?
Jamie> Hotplug scripts run as root to mount it, and you have your GUI
Jamie> / Shell Window which don't run as root to read and write a few
Jamie> of those files.
Jamie> You must not chown anything on the disk, because it isn't your
Jamie> disk.
Umm... this doesn't make sense. If it's not your disk, why are you
writing to it? And how is chown different?
The real answer is that your buddy should have setup a mode 777
directory on there where random stuff could be dropped. Again, a
userspace issue, not kernel.
>> But in that case, you're screwed anyway and it's going to become
>> un-manageable. Push this to userspace, not the kernel since it's a
>> userspace issue when you come right down to it.
Jamie> How do you handle the above scenario in userspace?
You certainly can't handle this in kernel space! If you just plug in
a random disk, and it comes up with borked UIDs because the original
server it came from has a different setup than yours for UID/GIDs,
then you're going to have to do *something* in a manual manner to fix
it.
Either you need to 'sudo chown -R user /media/disk/' or better yet
just mkdir a new directory with appropriate permissions and then write
using your GUI/shell-window, non-root user account to that new
directory.
Putting in a mount option like this is just begging for all kinds of
issues.
John
On Jul 25, 2009 17:44 +0200, Ludwig Nussel wrote:
> Andreas Dilger wrote:
> > 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.
>
> That's unnecessarily complicated. You don't have to keep track of
> your user ids when using e.g. FAT formatted USB memory sticks
> either. The files just always magically appear to be owned by the
> user who mounted the file system. The goal is to have it just as
> simple with ext2 on the USB stick.
But that isn't how this patch works either. It only makes files owned
by root available to the mounting user, and then (to add confusion)
files created by the user end up being owned by root. That means it
won't be a generally useful feature until every system also has this
patch. Also, by using root for the file owner you potentially expose
the system to more security risks compared to using any other user.
> If one of the systems doesn't mount media with the uid option the
> files might be unaccesible, that's true.
... but that is the whole point of this patch - to make files on the
device accessible between multiple systems, so if it doesn't do that
right out of the box it isn't a very useful feature.
> IOW on that system the situation is no different from
> today any you'll have to resort to the same workarounds you have to
> use today already (like sudo chown -R $USER or chmod 777).
But it also makes the problem worse, because the new files are owned
by root instead of either the UID on the original system or the UID
on the current system.
A more "obvious" solution would be to just have the filesystem mounted
with this option to make ALL files appear to be owned by the UID specified
to the "uid=${localuid}", which would at least more closely match the
behaviour of the fat/vfat filesystems with the uid= option. New files
could be created using the local UID with no more effort than creating
them with uid=0, but there would be less surprise on another system if
files don't magically appear as owned by root.
My further suggestion was that if it is possible to optionally specify
the remote UID then at least one side does not have to have this patch
in order to mount and use the filesystem. Hence, my suggestion to have
"uid={localuid}[={diskuid}]". The [={diskuid}] part could be optional,
and doesn't add any significant complication to the patch, AFAICS.
Initialize the diskuid = localuid, and if the additional diskuid is given
use that instead, for all inodes written to the disk.
Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.
Andreas Dilger wrote:
> [...]
> A more "obvious" solution would be to just have the filesystem mounted
> with this option to make ALL files appear to be owned by the UID specified
> to the "uid=${localuid}", which would at least more closely match the
> behaviour of the fat/vfat filesystems with the uid= option.
Well, I don't mind. The change to have it that way is trivial.
> New files
> could be created using the local UID with no more effort than creating
> them with uid=0, but there would be less surprise on another system if
> files don't magically appear as owned by root.
Sure, the disadvantage is that by using the local id the fs might
aggregate many different uids over time which looks weird on a
legacy system though.
> My further suggestion was that if it is possible to optionally specify
> the remote UID then at least one side does not have to have this patch
> in order to mount and use the filesystem. Hence, my suggestion to have
> "uid={localuid}[={diskuid}]". The [={diskuid}] part could be optional,
> and doesn't add any significant complication to the patch, AFAICS.
> Initialize the diskuid = localuid, and if the additional diskuid is given
> use that instead, for all inodes written to the disk.
Hmm, ok. Could be used to have all files root owned too.
cu
Ludwig
--
(o_ Ludwig Nussel
//\
V_/_ http://www.suse.de/
SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nuernberg)
Signed-off-by: Ludwig Nussel <[email protected]>
---
Documentation/filesystems/ext2.txt | 6 ++++++
fs/ext2/inode.c | 12 ++++++++++++
fs/ext2/super.c | 35 ++++++++++++++++++++++++++++++++++-
include/linux/ext2_fs_sb.h | 4 ++++
4 files changed, 56 insertions(+), 1 deletions(-)
diff --git a/Documentation/filesystems/ext2.txt b/Documentation/filesystems/ext2.txt
index 67639f9..5501ae0 100644
--- a/Documentation/filesystems/ext2.txt
+++ b/Documentation/filesystems/ext2.txt
@@ -42,6 +42,12 @@ orlov (*) Use the Orlov block allocator.
resuid=n The user ID which may use the reserved blocks.
resgid=n The group ID which may use the reserved blocks.
+uid=n[:m] make all files appear to belong to this is uid.
+ The optional second uid is actually written do disk.
+
+gid=n[:m] make all files appear to belong to this is gid.
+ The optional second gid is actually written do disk.
+
sb=n Use alternate superblock at this location.
user_xattr Enable "user." POSIX Extended Attributes
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index e271303..7ceb73c 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -1239,6 +1239,12 @@ struct inode *ext2_iget (struct super_block *sb, unsigned long ino)
inode->i_uid |= le16_to_cpu(raw_inode->i_uid_high) << 16;
inode->i_gid |= le16_to_cpu(raw_inode->i_gid_high) << 16;
}
+ if (EXT2_SB(sb)->s_uid) {
+ inode->i_uid = EXT2_SB(sb)->s_uid;
+ }
+ if (EXT2_SB(sb)->s_gid) {
+ inode->i_gid = EXT2_SB(sb)->s_gid;
+ }
inode->i_nlink = le16_to_cpu(raw_inode->i_links_count);
inode->i_size = le32_to_cpu(raw_inode->i_size);
inode->i_atime.tv_sec = (signed)le32_to_cpu(raw_inode->i_atime);
@@ -1353,6 +1359,12 @@ 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) {
+ uid = EXT2_SB(sb)->s_diskuid;
+ }
+ if (EXT2_SB(sb)->s_gid) {
+ gid = EXT2_SB(sb)->s_diskgid;
+ }
if (!(test_opt(sb, NO_UID32))) {
raw_inode->i_uid_low = cpu_to_le16(low_16_bits(uid));
raw_inode->i_gid_low = cpu_to_le16(low_16_bits(gid));
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 1a9ffee..f89b407 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -382,7 +382,8 @@ enum {
Opt_err_ro, Opt_nouid32, Opt_nocheck, Opt_debug,
Opt_oldalloc, Opt_orlov, Opt_nobh, Opt_user_xattr, Opt_nouser_xattr,
Opt_acl, Opt_noacl, Opt_xip, Opt_ignore, Opt_err, Opt_quota,
- Opt_usrquota, Opt_grpquota, Opt_reservation, Opt_noreservation
+ Opt_usrquota, Opt_grpquota, Opt_reservation, Opt_noreservation,
+ Opt_uid, Opt_diskuid, Opt_gid, Opt_diskgid
};
static const match_table_t tokens = {
@@ -416,6 +417,10 @@ static const match_table_t tokens = {
{Opt_usrquota, "usrquota"},
{Opt_reservation, "reservation"},
{Opt_noreservation, "noreservation"},
+ {Opt_uid, "uid=%u"},
+ {Opt_diskuid, "uid=%u:%u"},
+ {Opt_gid, "gid=%u"},
+ {Opt_diskgid, "gid=%u:%u"},
{Opt_err, NULL}
};
@@ -556,6 +561,34 @@ static int parse_options (char * options,
clear_opt(sbi->s_mount_opt, RESERVATION);
printk("reservations OFF\n");
break;
+ case Opt_uid:
+ if (match_int(&args[0], &option))
+ return 0;
+ sbi->s_uid = sbi->s_diskuid = option;
+ break;
+ case Opt_diskuid:
+ if (match_int(&args[0], &option))
+ return 0;
+ sbi->s_uid = option;
+
+ if (match_int(&args[1], &option))
+ return 0;
+ sbi->s_diskuid = option;
+ break;
+ case Opt_gid:
+ if (match_int(&args[0], &option))
+ return 0;
+ sbi->s_gid = sbi->s_diskgid = option;
+ break;
+ case Opt_diskgid:
+ if (match_int(&args[0], &option))
+ return 0;
+ sbi->s_gid = option;
+
+ if (match_int(&args[1], &option))
+ return 0;
+ sbi->s_diskgid = option;
+ break;
case Opt_ignore:
break;
default:
diff --git a/include/linux/ext2_fs_sb.h b/include/linux/ext2_fs_sb.h
index 1cdb663..062e8a9 100644
--- a/include/linux/ext2_fs_sb.h
+++ b/include/linux/ext2_fs_sb.h
@@ -88,6 +88,10 @@ struct ext2_sb_info {
unsigned long s_sb_block;
uid_t s_resuid;
gid_t s_resgid;
+ uid_t s_uid; /* make all files appear to belong to this uid */
+ uid_t s_diskuid; /* write this uid to disk (if s_uid != 0) */
+ gid_t s_gid; /* make all files appear to belong to this gid */
+ gid_t s_diskgid; /* write this gid to disk (if s_gid != 0) */
unsigned short s_mount_state;
unsigned short s_pad;
int s_addr_per_block_bits;
--
1.6.2.1
Signed-off-by: Ludwig Nussel <[email protected]>
---
Documentation/filesystems/ext3.txt | 6 ++++++
fs/ext3/inode.c | 26 ++++++++++++++++++++------
fs/ext3/super.c | 34 +++++++++++++++++++++++++++++++++-
include/linux/ext3_fs_sb.h | 4 ++++
4 files changed, 63 insertions(+), 7 deletions(-)
diff --git a/Documentation/filesystems/ext3.txt b/Documentation/filesystems/ext3.txt
index 570f9bd..84e0254 100644
--- a/Documentation/filesystems/ext3.txt
+++ b/Documentation/filesystems/ext3.txt
@@ -121,6 +121,12 @@ resgid=n The group ID which may use the reserved blocks.
resuid=n The user ID which may use the reserved blocks.
+uid=n[:m] make all files appear to belong to this is uid.
+ The optional second uid is actually written do disk.
+
+gid=n[:m] make all files appear to belong to this is gid.
+ The optional second gid is actually written do disk.
+
sb=n Use alternate superblock at this location.
quota
diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
index 5f51fed..afb5c53 100644
--- a/fs/ext3/inode.c
+++ b/fs/ext3/inode.c
@@ -2766,6 +2766,12 @@ struct inode *ext3_iget(struct super_block *sb, unsigned long ino)
inode->i_uid |= le16_to_cpu(raw_inode->i_uid_high) << 16;
inode->i_gid |= le16_to_cpu(raw_inode->i_gid_high) << 16;
}
+ if (EXT3_SB(sb)->s_uid) {
+ inode->i_uid = EXT3_SB(sb)->s_uid;
+ }
+ if (EXT3_SB(sb)->s_gid) {
+ inode->i_gid = EXT3_SB(sb)->s_gid;
+ }
inode->i_nlink = le16_to_cpu(raw_inode->i_links_count);
inode->i_size = le32_to_cpu(raw_inode->i_size);
inode->i_atime.tv_sec = (signed)le32_to_cpu(raw_inode->i_atime);
@@ -2895,6 +2901,8 @@ static int ext3_do_update_inode(handle_t *handle,
{
struct ext3_inode *raw_inode = ext3_raw_inode(iloc);
struct ext3_inode_info *ei = EXT3_I(inode);
+ uid_t uid = inode->i_uid;
+ gid_t gid = inode->i_gid;
struct buffer_head *bh = iloc->bh;
int err = 0, rc, block;
@@ -2905,27 +2913,33 @@ static int ext3_do_update_inode(handle_t *handle,
ext3_get_inode_flags(ei);
raw_inode->i_mode = cpu_to_le16(inode->i_mode);
+ if (EXT3_SB(inode->i_sb)->s_uid) {
+ uid = EXT3_SB(inode->i_sb)->s_diskuid;
+ }
+ if (EXT3_SB(inode->i_sb)->s_gid) {
+ gid = EXT3_SB(inode->i_sb)->s_diskgid;
+ }
if(!(test_opt(inode->i_sb, NO_UID32))) {
- raw_inode->i_uid_low = cpu_to_le16(low_16_bits(inode->i_uid));
- raw_inode->i_gid_low = cpu_to_le16(low_16_bits(inode->i_gid));
+ raw_inode->i_uid_low = cpu_to_le16(low_16_bits(uid));
+ raw_inode->i_gid_low = cpu_to_le16(low_16_bits(gid));
/*
* Fix up interoperability with old kernels. Otherwise, old inodes get
* re-used with the upper 16 bits of the uid/gid intact
*/
if(!ei->i_dtime) {
raw_inode->i_uid_high =
- cpu_to_le16(high_16_bits(inode->i_uid));
+ cpu_to_le16(high_16_bits(uid));
raw_inode->i_gid_high =
- cpu_to_le16(high_16_bits(inode->i_gid));
+ cpu_to_le16(high_16_bits(gid));
} else {
raw_inode->i_uid_high = 0;
raw_inode->i_gid_high = 0;
}
} else {
raw_inode->i_uid_low =
- cpu_to_le16(fs_high2lowuid(inode->i_uid));
+ cpu_to_le16(fs_high2lowuid(uid));
raw_inode->i_gid_low =
- cpu_to_le16(fs_high2lowgid(inode->i_gid));
+ cpu_to_le16(fs_high2lowgid(gid));
raw_inode->i_uid_high = 0;
raw_inode->i_gid_high = 0;
}
diff --git a/fs/ext3/super.c b/fs/ext3/super.c
index 524b349..eb22c82 100644
--- a/fs/ext3/super.c
+++ b/fs/ext3/super.c
@@ -779,7 +779,7 @@ enum {
Opt_usrjquota, Opt_grpjquota, Opt_offusrjquota, Opt_offgrpjquota,
Opt_jqfmt_vfsold, Opt_jqfmt_vfsv0, Opt_quota, Opt_noquota,
Opt_ignore, Opt_barrier, Opt_err, Opt_resize, Opt_usrquota,
- Opt_grpquota
+ Opt_grpquota, Opt_uid, Opt_diskuid, Opt_gid, Opt_diskgid
};
static const match_table_t tokens = {
@@ -832,6 +832,10 @@ static const match_table_t tokens = {
{Opt_usrquota, "usrquota"},
{Opt_barrier, "barrier=%u"},
{Opt_resize, "resize"},
+ {Opt_uid, "uid=%u"},
+ {Opt_diskuid, "uid=%u:%u"},
+ {Opt_gid, "gid=%u"},
+ {Opt_diskgid, "gid=%u:%u"},
{Opt_err, NULL},
};
@@ -1183,6 +1187,34 @@ set_qf_format:
case Opt_bh:
clear_opt(sbi->s_mount_opt, NOBH);
break;
+ case Opt_uid:
+ if (match_int(&args[0], &option))
+ return 0;
+ sbi->s_uid = sbi->s_diskuid = option;
+ break;
+ case Opt_diskuid:
+ if (match_int(&args[0], &option))
+ return 0;
+ sbi->s_uid = option;
+
+ if (match_int(&args[1], &option))
+ return 0;
+ sbi->s_diskuid = option;
+ break;
+ case Opt_gid:
+ if (match_int(&args[0], &option))
+ return 0;
+ sbi->s_gid = sbi->s_diskgid = option;
+ break;
+ case Opt_diskgid:
+ if (match_int(&args[0], &option))
+ return 0;
+ sbi->s_gid = option;
+
+ if (match_int(&args[1], &option))
+ return 0;
+ sbi->s_diskgid = option;
+ break;
default:
printk (KERN_ERR
"EXT3-fs: Unrecognized mount option \"%s\" "
diff --git a/include/linux/ext3_fs_sb.h b/include/linux/ext3_fs_sb.h
index f07f34d..2fa2b49 100644
--- a/include/linux/ext3_fs_sb.h
+++ b/include/linux/ext3_fs_sb.h
@@ -47,6 +47,10 @@ struct ext3_sb_info {
ext3_fsblk_t s_sb_block;
uid_t s_resuid;
gid_t s_resgid;
+ uid_t s_uid; /* make all files appear to belong to this uid */
+ uid_t s_diskuid; /* write this uid to disk (if s_uid != 0) */
+ gid_t s_gid; /* make all files appear to belong to this gid */
+ gid_t s_diskgid; /* write this gid to disk (if s_gid != 0) */
unsigned short s_mount_state;
unsigned short s_pad;
int s_addr_per_block_bits;
--
1.6.2.1
As suggested by Andreas Dilger, map _all_ files to the specified
uid and also store the specified uid on disk instead of using root.
Optionally the uid used on disk can be specified.
Additionally the gid can be changed too now in the same way as the
uid.
On Jul 28, 2009 13:01 +0200, Ludwig Nussel wrote:
> As suggested by Andreas Dilger, map _all_ files to the specified
> uid and also store the specified uid on disk instead of using root.
> Optionally the uid used on disk can be specified.
>
> Additionally the gid can be changed too now in the same way as the
> uid.
Looks good, you can add:
Acked-by: Andreas Dilger <[email protected]>
Please also submit the same patch for ext4.
Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.