Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extfs: debugfs rdump preserve flags #751

Closed
qkaiser opened this issue Feb 7, 2024 · 2 comments · Fixed by #772
Closed

extfs: debugfs rdump preserve flags #751

qkaiser opened this issue Feb 7, 2024 · 2 comments · Fixed by #772
Labels
bug Something isn't working

Comments

@qkaiser
Copy link
Contributor

qkaiser commented Feb 7, 2024

The Origins

I'm not exactly sure if the root of the problem lies in the file we got or in the way debugfs is implemented, but we end up with a directory that looks like this after debugfs stopped shouting at us that it failed changing ownership.

total 0
d????????? ? ? ? ?            ? .
d????????? ? ? ? ?            ? ..
-????????? ? ? ? ?            ? alt.png
-????????? ? ? ? ?            ? clipboard.png
-????????? ? ? ? ?            ? connect.png
-????????? ? ? ? ?            ? ctrlaltdel.png
-????????? ? ? ? ?            ? ctrl.png

Command "Design"

debugfs implements two dumping commands:

  • dump or dump_inode: works on files
  • rdump: works on directories

The two interfaces differ:

Usage: dump_inode [-p] <file> <output_file>
Usage: rdump <directory>... <native directory>

The -p parameter in dump_inode corresponds to preserve, a mode in which debugfs will do its best to preserve permissions and ownership of the files.

The problem is that rdump does not expose such a flag and implicitly considers the preserve flag to be set to 1. In rdump_inode, the following call is made:

if (dump_file("rdump", ino, fd, 1, fullname) != 0) {

With the definition of being:

static int dump_file(const char *cmdname, ext2_ino_t ino, int fd, int preserve, char *outname)

So we have two problems here:

  • we don't want to preserve permissions with unblob, because it leads to all kinds of noise with corrupted file systems or filesystems where all the files are owned by root
  • the call to dump_file done by rdump_inode with an implicit preserve flag is really bad design

Fix ?

A naive fix is this one:

diff --git a/debugfs/dump.c b/debugfs/dump.c
index b8a46eac..2ab84e7d 100644
--- a/debugfs/dump.c
+++ b/debugfs/dump.c
@@ -281,7 +281,7 @@ static void rdump_inode(ext2_ino_t ino, struct ext2_inode *inode,
                        com_err("rdump", errno, "while opening %s", fullname);
                        goto errout;
                }
-               if (dump_file("rdump", ino, fd, 1, fullname) != 0) {
+               if (dump_file("rdump", ino, fd, 0, fullname) != 0) {
       com_err("rdump", errno, "while dumping %s", fullname);
                        free(fullname);
       exit(1);
@@ -307,7 +307,7 @@ static void rdump_inode(ext2_ino_t ino, struct ext2_inode *inode,
                if (retval)
                        com_err("rdump", retval, "while dumping %s", fullname);
 
-               fix_perms("rdump", inode, -1, fullname);
+               //fix_perms("rdump", inode, -1, fullname);
        }
        /* else do nothing (don't dump device files, sockets, fifos, etc.) */

But the rdump interface must be modified to accept a preserve flag so that we can explicitly tell it not to preserve with unblob.

@qkaiser qkaiser added the bug Something isn't working label Feb 7, 2024
@qkaiser
Copy link
Contributor Author

qkaiser commented Feb 8, 2024

As indicated by the great @e3krisztian:

This can happen if we have r, but not x permission for a directory: its content can be enumerated (read) as names, but access to details (inode) is restricted.

@qkaiser
Copy link
Contributor Author

qkaiser commented Feb 9, 2024

Being taken care of at onekey-sec/e2fsprogs#8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant