tytso / e2fsprogs

Ext2/3/4 file system utilities
http://ext4.wiki.kernel.org
370 stars 218 forks source link

debugfs: Exit status is 0 even if some errors occurred #90

Open DemiMarie opened 2 years ago

DemiMarie commented 2 years ago

If a command stream passed (with -f) to debugfs contains a request to set an extended attribute on a nonexistent file, debugfs still exits with status 0. This is annoying when debugfs is used non-interactively.

tytso commented 2 years ago

Debugfs meant as a debugging tool. (Its original purpose was to corrupt file systems to create regression tests for e2fsck.) Over the years, more and more people have been attempting to use it in scripts, and it's not really well meant for that purpose. I did add the -R option to make it somewhat possible to use it in scripts, but ultimately, debugfs was meant to be used interactively, and it's a non-trivial addition to add exit status returns, since all of the debugfs commands would have to be modified, and we'd probably have to stash the the exit status in a global variable, since the ss library doesn't support exit status codes. (The ss library was derived from a design taken from Multics, and dates back to the 1980's, so it predates Linux, and that which it took its design inspiration predates Unix. :-)

The way Android creates its system images is via a C program (see contrib/android for the sources to e2fsdroid). It's pretty specialized for Android, since it needs to create SELinux security labels as well as extended attributes, and it uses libsparse (found in the AOSP sources, and not packaged in most dstributions besides Debian) to write out system images in a sparse format used by Android tools.

The mke2fs program has the -d option, which has been enough for simple build systems creating images, but it certainly has its flaws. One is that if you want to create setuid root files, you either need to have root privileges, or use an ldpreload hack such as Debian's fakeroot. Another is that while it does support extended attributes and Posix ACL's, it doesn't support SELinux labels, or other more exotic xattrs. And finally, as you have pointed out in other issues, it currently isn't possible to control the last write time (although it would be pretty easy to support for something like the E2FSCK_TIME environment variable to mke2fs to set fs->now to a particular time).

I don't have time to implement this, but what would probably make sense is to have some way of passing in a config file (perhaps using a syntax similar to e2fsck.conf and mke2fs.conf --- the parser for this INI-style files is in lib/support/profile.[ch]) which allows the specification of the user and group ownership for files to override the value in directory, and also allows the specification things like SELinux labels as needed. We could also have a way to allow the creation of device nodes without needign the build system from having root privs (or using a Debian-style fakeroot scheme).

Past improvements to mke2fs -d have come when people who are building embedded systems contribute patches to enhance its functionality. If you're interested in working on this with me, that would be great!

Another, perhaps more general way, would be to work on creating SWIG bindings for libext2fs, so that someone could do something like e2fsdroid, but using Python or Perl, which might be a bit more convenient for making scripted changes to a library without trying to inject commands script results to and from debugfs.

If either of these are itches you'd like to scratch, let me know!

DemiMarie commented 2 years ago

SELinux labels are extended attributes, so a solution that solves one problem will solve the other.

I think the simplest approach is:

tytso commented 2 years ago

On Mon, Dec 06, 2021 at 12:40:03PM -0800, Demi Marie Obenour wrote:

  • Provide a flag to set all of the timestamps, either on just the filesystem, or also on each file in the filesystem.

We can debate whether this is done via an extended option ("mke2fs -E force_timestamp=XXX") or via an environment variable, or via a configuration file, but sure.

  • Allow forcibly setting the uid and gid of all inodes to root:root (this is a common special case) unless overridden elsewhere.

What did you have in mind with "unless overridden elsewhere"?

  • Read all of the other metadata from user.* extended attributes, which don’t require special privileges to modify. For instance, security.selinux could be read from user.ext4.xattr.security.selinux, the UID from user.ext4.uid, the GID from user.ext4.gid, and so on.

There is a potential problem with this, whch is that "user.ext4.xattr.security.linux" takes up significantly more room than "security.linux" (since "security" is encoded using a 1 byte namespace id, and the on-disk only needs space for 6 bytes, as opposed to the significantly longer bytes needed to encode "user.ext4.xattr.security.linux"

The security xattrs, since they are meant for use by the kernel, are binary encoded, so setting them via shell script is not going to be easy --- and if you're going to be using a C program, you might as well call libext2fs directly, ala e2fsdroid.

So if you want to be able to encode a large number special case overrides as xattrs, since ext4 requires that all of the extended attributes (names and values, plus some additional metadata) must fit within a file system block (normally 4k), encoding this configuration in the extended attributes in the xattrs could run into this limit.

Also, once we consider the complexity needed to set all of the extended attributes into directory hierarchy which gets passed to mke2fs via the -d option, in the long run, I suspect it will be much simpler for the build systems which are creating the file system image to read the file metadata from a single file (which can be checked into git or some other source code management system).

So for example, what I was thinking of was a config file that might look like this:

[users] root = 0 tytso = 1000

[groups] tytso = 1000 wheel = 10

[defaults] / = { uid = 0 gid = 0 dir_mode = 9755 file_mode = 0444 time = 19990102 } /bin = { file_mode = 0755 } /home/tytso = { uid = tytso gid = tytso dir_mode = 0750 file_mode = 640 }

[overrides] /bin/sudo = { uid = 0 gid = wheel mode = 04750 }