[PATCH] btrfs: remove runtime tweakable feature sysfs interface

Qu Wenruo <[email protected]>
Newsgroups org.kernel.vger.linux-btrfs
Message-ID <8a598d76555b5944d34bb08fa8dbeea28fc05db9.1787307129.git.wqu@suse.com>
There are 2 features that are marked runtime tweakable inside
/sys/fs/btrfs/features/

- acl
  Which is a mount option, and it will not show up in
  /sys/fs/btrfs/<fsid>/features/ directory anyway.

- extended_iref
  This feature can only be enabled, but not disabled at runtime.
  Furthermore it's already the default behavior since 3.12.

  So it means this feature is always enabled and cannot be disabled for
  modern btrfs.

So there is no need to maintain the ability to modify btrfs' runtime
features through sysfs.

And furthermore, the existing btrfs_feature_attr_store() is race-prone,
it relies on fs_info->transaction_kthread, but our sysfs interfaces are
enabled before transaction_kthread.

Meaning at mount time a sysfs write can trigger NULL pointer dereference
if the transaction_kthread is not yet initialized.
The opposite is also possible during unmount.

Thankfully that race is not possible in the real world, as the only
supported feature is already enabled.

But it also means we do not really need to keep the race-prone
infrastructure, so just remove it completely, and make the per-module
and per-mount features files to be completely read-only.

Even with the sysfs tweakable features removed, we can still enable
extended_iref feature through ioctl.

Signed-off-by: Qu Wenruo <[email protected]>
---
 fs/btrfs/sysfs.c | 121 ++---------------------------------------------
 1 file changed, 4 insertions(+), 117 deletions(-)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 39cb01ee441a..1f78bb1cf813 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -83,8 +83,7 @@ struct raid_kobject {
 #define BTRFS_FEAT_ATTR(_name, _feature_set, _feature_prefix, _feature_bit)  \
 static struct btrfs_feature_attr btrfs_attr_features_##_name = {	     \
 	.kobj_attr = __INIT_KOBJ_ATTR(_name, S_IRUGO,			     \
-				      btrfs_feature_attr_show,		     \
-				      btrfs_feature_attr_store),	     \
+				      btrfs_feature_attr_show, NULL),	     \
 	.feature_set	= _feature_set,					     \
 	.feature_bit	= _feature_prefix ##_## _feature_bit,		     \
 }
@@ -130,132 +129,22 @@ static u64 get_features(struct btrfs_fs_info *fs_info,
 		return btrfs_super_incompat_flags(disk_super);
 }
 
-static void set_features(struct btrfs_fs_info *fs_info,
-			 enum btrfs_feature_set set, u64 features)
-{
-	struct btrfs_super_block *disk_super = fs_info->super_copy;
-	if (set == FEAT_COMPAT)
-		btrfs_set_super_compat_flags(disk_super, features);
-	else if (set == FEAT_COMPAT_RO)
-		btrfs_set_super_compat_ro_flags(disk_super, features);
-	else
-		btrfs_set_super_incompat_flags(disk_super, features);
-}
-
-static int can_modify_feature(struct btrfs_feature_attr *fa)
-{
-	int val = 0;
-	u64 set, clear;
-	switch (fa->feature_set) {
-	case FEAT_COMPAT:
-		set = BTRFS_FEATURE_COMPAT_SAFE_SET;
-		clear = BTRFS_FEATURE_COMPAT_SAFE_CLEAR;
-		break;
-	case FEAT_COMPAT_RO:
-		set = BTRFS_FEATURE_COMPAT_RO_SAFE_SET;
-		clear = BTRFS_FEATURE_COMPAT_RO_SAFE_CLEAR;
-		break;
-	case FEAT_INCOMPAT:
-		set = BTRFS_FEATURE_INCOMPAT_SAFE_SET;
-		clear = BTRFS_FEATURE_INCOMPAT_SAFE_CLEAR;
-		break;
-	default:
-		btrfs_warn(NULL, "sysfs: unknown feature set %d", fa->feature_set);
-		return 0;
-	}
-
-	if (set & fa->feature_bit)
-		val |= 1;
-	if (clear & fa->feature_bit)
-		val |= 2;
-
-	return val;
-}
-
 static ssize_t btrfs_feature_attr_show(struct kobject *kobj,
 				       struct kobj_attribute *a, char *buf)
 {
 	int val = 0;
 	struct btrfs_fs_info *fs_info = to_fs_info(kobj);
 	struct btrfs_feature_attr *fa = to_btrfs_feature_attr(a);
+
 	if (fs_info) {
 		u64 features = get_features(fs_info, fa->feature_set);
 		if (features & fa->feature_bit)
 			val = 1;
-	} else
-		val = can_modify_feature(fa);
+	}
 
 	return sysfs_emit(buf, "%d\n", val);
 }
 
-static ssize_t btrfs_feature_attr_store(struct kobject *kobj,
-					struct kobj_attribute *a,
-					const char *buf, size_t count)
-{
-	struct btrfs_fs_info *fs_info;
-	struct btrfs_feature_attr *fa = to_btrfs_feature_attr(a);
-	u64 features, set, clear;
-	unsigned long val;
-	int ret;
-
-	fs_info = to_fs_info(kobj);
-	if (!fs_info)
-		return -EPERM;
-
-	if (sb_rdonly(fs_info->sb))
-		return -EROFS;
-
-	ret = kstrtoul(skip_spaces(buf), 0, &val);
-	if (ret)
-		return ret;
-
-	if (fa->feature_set == FEAT_COMPAT) {
-		set = BTRFS_FEATURE_COMPAT_SAFE_SET;
-		clear = BTRFS_FEATURE_COMPAT_SAFE_CLEAR;
-	} else if (fa->feature_set == FEAT_COMPAT_RO) {
-		set = BTRFS_FEATURE_COMPAT_RO_SAFE_SET;
-		clear = BTRFS_FEATURE_COMPAT_RO_SAFE_CLEAR;
-	} else {
-		set = BTRFS_FEATURE_INCOMPAT_SAFE_SET;
-		clear = BTRFS_FEATURE_INCOMPAT_SAFE_CLEAR;
-	}
-
-	features = get_features(fs_info, fa->feature_set);
-
-	/* Nothing to do */
-	if ((val && (features & fa->feature_bit)) ||
-	    (!val && !(features & fa->feature_bit)))
-		return count;
-
-	if ((val && !(set & fa->feature_bit)) ||
-	    (!val && !(clear & fa->feature_bit))) {
-		btrfs_info(fs_info,
-			"%sabling feature %s on mounted fs is not supported.",
-			val ? "En" : "Dis", fa->kobj_attr.attr.name);
-		return -EPERM;
-	}
-
-	btrfs_info(fs_info, "%s %s feature flag",
-		   val ? "Setting" : "Clearing", fa->kobj_attr.attr.name);
-
-	spin_lock(&fs_info->super_lock);
-	features = get_features(fs_info, fa->feature_set);
-	if (val)
-		features |= fa->feature_bit;
-	else
-		features &= ~fa->feature_bit;
-	set_features(fs_info, fa->feature_set, features);
-	spin_unlock(&fs_info->super_lock);
-
-	/*
-	 * We don't want to do full transaction commit from inside sysfs
-	 */
-	set_bit(BTRFS_FS_NEED_TRANS_COMMIT, &fs_info->flags);
-	wake_up_process(fs_info->transaction_kthread);
-
-	return count;
-}
-
 static umode_t btrfs_feature_visible(struct kobject *kobj,
 				     struct attribute *attr, int unused)
 {
@@ -269,9 +158,7 @@ static umode_t btrfs_feature_visible(struct kobject *kobj,
 		fa = attr_to_btrfs_feature_attr(attr);
 		features = get_features(fs_info, fa->feature_set);
 
-		if (can_modify_feature(fa))
-			mode |= S_IWUSR;
-		else if (!(features & fa->feature_bit))
+		if (!(features & fa->feature_bit))
 			mode = 0;
 	}
 
-- 
2.54.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.