[PATCH] mke2fs: do not change root dir ownership

[PATCH] mke2fs: do not change root dir ownership

[Mike Frysinger]

If you use `mke2fs` on a file, the code will automatically chown the root
dir to the active uid/gid.  It doesn't do this to any other files though.

I can't see where this would really be desirable: you still need root in
order to mount, and the lost+found dir is owned by root.  It means if you
want to generate a rootfs as a non-root user, you first have to run it
through sudo or manually run `chown 0:0` after you've mounted it.

I'm not aware of other tools that do this (in fact, tools tend to do the
opposite thing -- squash the uid/gid to 0/0 so that you can generate the
fs as no-root), so punt it.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
not sure this is worth writing a command line flag for ...

 misc/mke2fs.c | 24 ------------------------
 1 file changed, 24 deletions(-)

diff --git a/misc/mke2fs.c b/misc/mke2fs.c
index 7ff759d..30767d8 100644
--- a/misc/mke2fs.c
+++ b/misc/mke2fs.c
@@ -383,36 +383,12 @@ static void write_inode_tables(ext2_filsys fs, int lazy_flag, int itable_zeroed)
 static void create_root_dir(ext2_filsys fs)
 {
 	errcode_t		retval;
-	struct ext2_inode	inode;
-	__u32			uid, gid;
 
 	retval = ext2fs_mkdir(fs, EXT2_ROOT_INO, EXT2_ROOT_INO, 0);
 	if (retval) {
 		com_err("ext2fs_mkdir", retval, _("while creating root dir"));
 		exit(1);
 	}
-	if (geteuid()) {
-		retval = ext2fs_read_inode(fs, EXT2_ROOT_INO, &inode);
-		if (retval) {
-			com_err("ext2fs_read_inode", retval,
-				_("while reading root inode"));
-			exit(1);
-		}
-		uid = getuid();
-		inode.i_uid = uid;
-		ext2fs_set_i_uid_high(inode, uid >> 16);
-		if (uid) {
-			gid = getgid();
-			inode.i_gid = gid;
-			ext2fs_set_i_gid_high(inode, gid >> 16);
-		}
-		retval = ext2fs_write_new_inode(fs, EXT2_ROOT_INO, &inode);
-		if (retval) {
-			com_err("ext2fs_write_inode", retval,
-				_("while setting root inode ownership"));
-			exit(1);
-		}
-	}
 }
 
 static void create_lost_and_found(ext2_filsys fs)
-- 
1.8.2.1

Re: [PATCH] mke2fs: do not change root dir ownership

[Theodore Ts'o]

On Mon, May 06, 2013 at 05:21:56PM -0400, Mike Frysinger wrote:
> If you use `mke2fs` on a file, the code will automatically chown the root
> dir to the active uid/gid.  It doesn't do this to any other files though.
> 
> I can't see where this would really be desirable: you still need root in
> order to mount, and the lost+found dir is owned by root.  It means if you
> want to generate a rootfs as a non-root user, you first have to run it
> through sudo or manually run `chown 0:0` after you've mounted it.

Yeah, this was something that we've been doing in e2fsprogs since 0.5b
(i.e., dating back to 1997).  I agree that the behavior is a bit silly
and we should probably change it.  It *is* a behavioural change,
though, so I'm going to make it something that changes in 1.43, as
opposed to a 1.42.x maintenance release.

A workarond that I'd recommend (since we will have lots of people
creating file systems for various mobile/embedded systems, and they
will have scripts that need to work on existing versions of e2fsprogs)
is to do something like this:

   mke2fs -t ext4 /tmp/foo.img 16384
   debugfs /tmp/foo.img -R "set_inode_field / uid 0"
   debugfs /tmp/foo.img -R "set_inode_field / gid 0"

   	   		   		      	  - Ted

Re: [PATCH] mke2fs: do not change root dir ownership

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 1590 bytes --]

On Monday 13 May 2013 09:37:31 Theodore Ts'o wrote:
> On Mon, May 06, 2013 at 05:21:56PM -0400, Mike Frysinger wrote:
> > If you use `mke2fs` on a file, the code will automatically chown the root
> > dir to the active uid/gid.  It doesn't do this to any other files though.
> > 
> > I can't see where this would really be desirable: you still need root in
> > order to mount, and the lost+found dir is owned by root.  It means if you
> > want to generate a rootfs as a non-root user, you first have to run it
> > through sudo or manually run `chown 0:0` after you've mounted it.
> 
> Yeah, this was something that we've been doing in e2fsprogs since 0.5b
> (i.e., dating back to 1997).  I agree that the behavior is a bit silly
> and we should probably change it.  It *is* a behavioural change,
> though, so I'm going to make it something that changes in 1.43, as
> opposed to a 1.42.x maintenance release.

np.  as long as it eventually gets fixed, i'm happy :).

> A workarond that I'd recommend (since we will have lots of people
> creating file systems for various mobile/embedded systems, and they
> will have scripts that need to work on existing versions of e2fsprogs)
> is to do something like this:
> 
>    mke2fs -t ext4 /tmp/foo.img 16384
>    debugfs /tmp/foo.img -R "set_inode_field / uid 0"
>    debugfs /tmp/foo.img -R "set_inode_field / gid 0"

nifty.  we already have to mount the fs to populate it, so putting a chown in 
there isn't a big deal in the current setup.

now if only e2fsprogs would integrate the `genext2fs` project :).
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mke2fs: do not change root dir ownership

[Theodore Ts'o]

On Mon, May 13, 2013 at 12:12:27PM -0400, Mike Frysinger wrote:
> nifty.  we already have to mount the fs to populate it, so putting a chown in 
> there isn't a big deal in the current setup.
> 
> now if only e2fsprogs would integrate the `genext2fs` project :).

Darren Hart created a shell script which uses debugfs to do all of the
dirty work, called populate-extfs.sh which I believe Linaro and the
Yocto project is using instead of genext2fs.

I'm not entirely sure this the latest version, but so you can get a
flavor of the script, see:

http://patches.openembedded.org/patch/45415/

Darren, if you think the script is pretty stable, I'd be happy to drop
the latest version of it into the contrib directory of e2fsprogs.

Cheers,

					- Ted

Re: [PATCH] mke2fs: do not change root dir ownership

[Darren Hart]

On 05/13/2013 02:06 PM, Theodore Ts'o wrote:
> On Mon, May 13, 2013 at 12:12:27PM -0400, Mike Frysinger wrote:
>> nifty.  we already have to mount the fs to populate it, so putting a chown in 
>> there isn't a big deal in the current setup.
>>
>> now if only e2fsprogs would integrate the `genext2fs` project :).
> 
> Darren Hart created a shell script which uses debugfs to do all of the
> dirty work, called populate-extfs.sh which I believe Linaro and the
> Yocto project is using instead of genext2fs.
> 
> I'm not entirely sure this the latest version, but so you can get a
> flavor of the script, see:
> 
> http://patches.openembedded.org/patch/45415/
> 
> Darren, if you think the script is pretty stable, I'd be happy to drop
> the latest version of it into the contrib directory of e2fsprogs.

Let me sync with Robert (on Cc) who has been doing the integration to
see what he thinks about that.

We consider the script to be a temporary mechanism. Now that the symlink
support has been added to debugfs, we are working to validate that
everything works via the script. Once complete, the second stage of this
effort is to move some of the logic from debugfs into libe2fs and make
it available to mke2fs as well as debugfs. Then we would want to add the
-i (initial dir) argument I had proposed originally to be able to format
and populate an image in a single step.

I provide that context in case it impacts Mike's plans. Always good to
know what others are planning :-)

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Technical Lead - Linux Kernel

Re: [PATCH] mke2fs: do not change root dir ownership

[Robert Yang]

On 05/14/2013 07:09 AM, Darren Hart wrote:
>
>
> On 05/13/2013 02:06 PM, Theodore Ts'o wrote:
>> On Mon, May 13, 2013 at 12:12:27PM -0400, Mike Frysinger wrote:
>>> nifty.  we already have to mount the fs to populate it, so putting a chown in
>>> there isn't a big deal in the current setup.
>>>
>>> now if only e2fsprogs would integrate the `genext2fs` project :).
>>
>> Darren Hart created a shell script which uses debugfs to do all of the
>> dirty work, called populate-extfs.sh which I believe Linaro and the
>> Yocto project is using instead of genext2fs.
>>
>> I'm not entirely sure this the latest version, but so you can get a
>> flavor of the script, see:
>>
>> http://patches.openembedded.org/patch/45415/
>>
>> Darren, if you think the script is pretty stable, I'd be happy to drop
>> the latest version of it into the contrib directory of e2fsprogs.
>
> Let me sync with Robert (on Cc) who has been doing the integration to
> see what he thinks about that.
>

Hi Darren, thanks for the info, the recent patches are here:

http://patches.openembedded.org/patch/49409/
http://patches.openembedded.org/patch/49413/
http://patches.openembedded.org/patch/49415/

It seem that they are still not stable enough from the replying of the oe-core
mailing list, and I'm working it.

// Robert

> We consider the script to be a temporary mechanism. Now that the symlink
> support has been added to debugfs, we are working to validate that
> everything works via the script. Once complete, the second stage of this
> effort is to move some of the logic from debugfs into libe2fs and make
> it available to mke2fs as well as debugfs. Then we would want to add the
> -i (initial dir) argument I had proposed originally to be able to format
> and populate an image in a single step.
>
> I provide that context in case it impacts Mike's plans. Always good to
> know what others are planning :-)
>