[PATCH v3] debugfs: Only clobber mode/uid/gid on remount if asked

From: Brian Norris
Date: Mon Sep 12 2022 - 19:32:00 EST


Users may have explicitly configured their debugfs permissions; we
shouldn't overwrite those just because a second mount appeared.

Only clobber if the options were provided at mount time.

Existing behavior:

## Pre-existing status: debugfs is 0755.
# chmod 755 /sys/kernel/debug/
# stat -c '%A' /sys/kernel/debug/
drwxr-xr-x

## New mount sets kernel-default permissions:
# mount -t debugfs none /mnt/foo
# stat -c '%A' /mnt/foo
drwx------

## Unexpected: the original mount changed permissions:
# stat -c '%A' /sys/kernel/debug
drwx------

New behavior:

## Pre-existing status: debugfs is 0755.
# chmod 755 /sys/kernel/debug/
# stat -c '%A' /sys/kernel/debug/
drwxr-xr-x

## New mount inherits existing permissions:
# mount -t debugfs none /mnt/foo
# stat -c '%A' /mnt/foo
drwxr-xr-x

## Expected: old mount is unchanged:
# stat -c '%A' /sys/kernel/debug
drwxr-xr-x

Full test cases are being submitted to LTP.

Signed-off-by: Brian Norris <briannorris@xxxxxxxxxxxx>
---
Temporary LTP branch here:
https://github.com/computersforpeace/ltp/commits/debugfs

I'll put in an LTP pull request once the kernel tree actually contains
my patches.

Companion patch:
tracefs: Only clobber mode/uid/gid on remount if asked
https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git/commit/?h=trace-v6.0-rc4&id=47311db8e8f33011d90dee76b39c8886120cdda4

Changes in v3:
* Factor out _debugfs_apply_options() to hide boolean arg
* Drop unused return value

Changes in v2:
* Update commit description
* Include LTP patch in notes
* Split from tracefs patch

fs/debugfs/inode.c | 37 ++++++++++++++++++++++++++++++-------
1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 3dcf0b8b4e93..83749cd67e0d 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -82,6 +82,8 @@ struct debugfs_mount_opts {
kuid_t uid;
kgid_t gid;
umode_t mode;
+ /* Opt_* bitfield. */
+ unsigned int opts;
};

enum {
@@ -111,6 +113,7 @@ static int debugfs_parse_options(char *data, struct debugfs_mount_opts *opts)
kgid_t gid;
char *p;

+ opts->opts = 0;
opts->mode = DEBUGFS_DEFAULT_MODE;

while ((p = strsep(&data, ",")) != NULL) {
@@ -145,24 +148,44 @@ static int debugfs_parse_options(char *data, struct debugfs_mount_opts *opts)
* but traditionally debugfs has ignored all mount options
*/
}
+
+ opts->opts |= BIT(token);
}

return 0;
}

-static int debugfs_apply_options(struct super_block *sb)
+static void _debugfs_apply_options(struct super_block *sb, bool remount)
{
struct debugfs_fs_info *fsi = sb->s_fs_info;
struct inode *inode = d_inode(sb->s_root);
struct debugfs_mount_opts *opts = &fsi->mount_opts;

- inode->i_mode &= ~S_IALLUGO;
- inode->i_mode |= opts->mode;
+ /*
+ * On remount, only reset mode/uid/gid if they were provided as mount
+ * options.
+ */

- inode->i_uid = opts->uid;
- inode->i_gid = opts->gid;
+ if (!remount || opts->opts & BIT(Opt_mode)) {
+ inode->i_mode &= ~S_IALLUGO;
+ inode->i_mode |= opts->mode;
+ }

- return 0;
+ if (!remount || opts->opts & BIT(Opt_uid))
+ inode->i_uid = opts->uid;
+
+ if (!remount || opts->opts & BIT(Opt_gid))
+ inode->i_gid = opts->gid;
+}
+
+static void debugfs_apply_options(struct super_block *sb)
+{
+ _debugfs_apply_options(sb, false);
+}
+
+static void debugfs_apply_options_remount(struct super_block *sb)
+{
+ _debugfs_apply_options(sb, true);
}

static int debugfs_remount(struct super_block *sb, int *flags, char *data)
@@ -175,7 +198,7 @@ static int debugfs_remount(struct super_block *sb, int *flags, char *data)
if (err)
goto fail;

- debugfs_apply_options(sb);
+ debugfs_apply_options_remount(sb);

fail:
return err;
--
2.37.2.789.g6183377224-goog