From 1f6087e69c73b2ce8f1c20a4199488d937bb6b67 Mon Sep 17 00:00:00 2001 From: Anand Jain Date: Wed, 12 Feb 2020 17:28:12 +0800 Subject: btrfs: sysfs, use btrfs_sysfs_remove_fsid to celanup errors in add_fsid We have one simple function btrfs_sysfs_remove_fsid() to undo btrfs_sysfs_add_fsid(), which also does proper checks before releasing objects. One difference, if btrfs_sysfs_remove_fsid is used that now we also call kobject_del() which was missing before. This was tested (with kobject debug turned on) and no change in behaviour was found. This is a cleanup patch. Signed-off-by: Anand Jain Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/btrfs/sysfs.c') diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c index 3c10e78924d0..119edd4341d6 100644 --- a/fs/btrfs/sysfs.c +++ b/fs/btrfs/sysfs.c @@ -1371,7 +1371,7 @@ int btrfs_sysfs_add_fsid(struct btrfs_fs_devices *fs_devs) if (!fs_devs->devices_kobj) { btrfs_err(fs_devs->fs_info, "failed to init sysfs device interface"); - kobject_put(&fs_devs->fsid_kobj); + btrfs_sysfs_remove_fsid(fs_devs); return -ENOMEM; } -- cgit v1.2.3 From f3cd2c58110dad14ee37cc47fd1473d90ee68ccb Mon Sep 17 00:00:00 2001 From: Anand Jain Date: Wed, 12 Feb 2020 17:28:13 +0800 Subject: btrfs: sysfs, rename device_link add/remove functions Since commit 668e48af7a94 ("btrfs: sysfs, add devid/dev_state kobject and device attributes"), the functions btrfs_sysfs_add_device_link() and btrfs_sysfs_rm_device_link() do more than just adding and removing the device link as its name indicated. Rename them to be more specific that's about the directory with the attirbutes Signed-off-by: Anand Jain Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/dev-replace.c | 4 ++-- fs/btrfs/sysfs.c | 10 +++++----- fs/btrfs/sysfs.h | 4 ++-- fs/btrfs/volumes.c | 8 ++++---- 4 files changed, 13 insertions(+), 13 deletions(-) (limited to 'fs/btrfs/sysfs.c') diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c index 2aad07cdaea8..db93909b25e0 100644 --- a/fs/btrfs/dev-replace.c +++ b/fs/btrfs/dev-replace.c @@ -512,7 +512,7 @@ static int btrfs_dev_replace_start(struct btrfs_fs_info *fs_info, atomic64_set(&dev_replace->num_uncorrectable_read_errors, 0); up_write(&dev_replace->rwsem); - ret = btrfs_sysfs_add_device_link(tgt_device->fs_devices, tgt_device); + ret = btrfs_sysfs_add_devices_dir(tgt_device->fs_devices, tgt_device); if (ret) btrfs_err(fs_info, "kobj add dev failed %d", ret); @@ -743,7 +743,7 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info, mutex_unlock(&fs_info->fs_devices->device_list_mutex); /* replace the sysfs entry */ - btrfs_sysfs_rm_device_link(fs_info->fs_devices, src_device); + btrfs_sysfs_remove_devices_dir(fs_info->fs_devices, src_device); btrfs_sysfs_update_devid(tgt_device); btrfs_rm_dev_replace_free_srcdev(src_device); diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c index 119edd4341d6..651aa65b1f3f 100644 --- a/fs/btrfs/sysfs.c +++ b/fs/btrfs/sysfs.c @@ -960,7 +960,7 @@ void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info) addrm_unknown_feature_attrs(fs_info, false); sysfs_remove_group(&fs_info->fs_devices->fsid_kobj, &btrfs_feature_attr_group); sysfs_remove_files(&fs_info->fs_devices->fsid_kobj, btrfs_attrs); - btrfs_sysfs_rm_device_link(fs_info->fs_devices, NULL); + btrfs_sysfs_remove_devices_dir(fs_info->fs_devices, NULL); } static const char * const btrfs_feature_set_names[FEAT_MAX] = { @@ -1149,7 +1149,7 @@ int btrfs_sysfs_add_space_info_type(struct btrfs_fs_info *fs_info, /* when one_device is NULL, it removes all device links */ -int btrfs_sysfs_rm_device_link(struct btrfs_fs_devices *fs_devices, +int btrfs_sysfs_remove_devices_dir(struct btrfs_fs_devices *fs_devices, struct btrfs_device *one_device) { struct hd_struct *disk; @@ -1269,7 +1269,7 @@ static struct kobj_type devid_ktype = { .release = btrfs_release_devid_kobj, }; -int btrfs_sysfs_add_device_link(struct btrfs_fs_devices *fs_devices, +int btrfs_sysfs_add_devices_dir(struct btrfs_fs_devices *fs_devices, struct btrfs_device *one_device) { int error = 0; @@ -1395,13 +1395,13 @@ int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info) btrfs_set_fs_info_ptr(fs_info); - error = btrfs_sysfs_add_device_link(fs_devs, NULL); + error = btrfs_sysfs_add_devices_dir(fs_devs, NULL); if (error) return error; error = sysfs_create_files(fsid_kobj, btrfs_attrs); if (error) { - btrfs_sysfs_rm_device_link(fs_devs, NULL); + btrfs_sysfs_remove_devices_dir(fs_devs, NULL); return error; } diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h index c68582add92e..718a26c97833 100644 --- a/fs/btrfs/sysfs.h +++ b/fs/btrfs/sysfs.h @@ -14,9 +14,9 @@ enum btrfs_feature_set { char *btrfs_printable_features(enum btrfs_feature_set set, u64 flags); const char * const btrfs_feature_set_name(enum btrfs_feature_set set); -int btrfs_sysfs_add_device_link(struct btrfs_fs_devices *fs_devices, +int btrfs_sysfs_add_devices_dir(struct btrfs_fs_devices *fs_devices, struct btrfs_device *one_device); -int btrfs_sysfs_rm_device_link(struct btrfs_fs_devices *fs_devices, +int btrfs_sysfs_remove_devices_dir(struct btrfs_fs_devices *fs_devices, struct btrfs_device *one_device); int btrfs_sysfs_add_fsid(struct btrfs_fs_devices *fs_devs); void btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs); diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index b092021e41e9..387f80656476 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -2054,7 +2054,7 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path, if (device->bdev) { cur_devices->open_devices--; /* remove sysfs entry */ - btrfs_sysfs_rm_device_link(fs_devices, device); + btrfs_sysfs_remove_devices_dir(fs_devices, device); } num_devices = btrfs_super_num_devices(fs_info->super_copy) - 1; @@ -2174,7 +2174,7 @@ void btrfs_destroy_dev_replace_tgtdev(struct btrfs_device *tgtdev) mutex_lock(&fs_devices->device_list_mutex); - btrfs_sysfs_rm_device_link(fs_devices, tgtdev); + btrfs_sysfs_remove_devices_dir(fs_devices, tgtdev); if (tgtdev->bdev) fs_devices->open_devices--; @@ -2522,7 +2522,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path orig_super_num_devices + 1); /* add sysfs device entry */ - btrfs_sysfs_add_device_link(fs_devices, device); + btrfs_sysfs_add_devices_dir(fs_devices, device); /* * we've got more storage, clear any full flags on the space @@ -2590,7 +2590,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path return ret; error_sysfs: - btrfs_sysfs_rm_device_link(fs_devices, device); + btrfs_sysfs_remove_devices_dir(fs_devices, device); mutex_lock(&fs_info->fs_devices->device_list_mutex); mutex_lock(&fs_info->chunk_mutex); list_del_rcu(&device->dev_list); -- cgit v1.2.3 From 25864778bce70341aad0fe65e9f5429370bbbbb2 Mon Sep 17 00:00:00 2001 From: Anand Jain Date: Thu, 13 Feb 2020 16:40:53 +0800 Subject: btrfs: sysfs, unify handler name of devinfo/missing The devinfo attribute handlers were added in 668e48af7a94 ("btrfs: sysfs, add devid/dev_state kobject and device attributes") and the name should contain _devinfo_, there's one that does not conform, so unify it with the rest. Signed-off-by: Anand Jain Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/sysfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs/btrfs/sysfs.c') diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c index 651aa65b1f3f..93cf76118a04 100644 --- a/fs/btrfs/sysfs.c +++ b/fs/btrfs/sysfs.c @@ -1205,7 +1205,7 @@ static ssize_t btrfs_devinfo_in_fs_metadata_show(struct kobject *kobj, } BTRFS_ATTR(devid, in_fs_metadata, btrfs_devinfo_in_fs_metadata_show); -static ssize_t btrfs_sysfs_missing_show(struct kobject *kobj, +static ssize_t btrfs_devinfo_missing_show(struct kobject *kobj, struct kobj_attribute *a, char *buf) { int val; @@ -1216,7 +1216,7 @@ static ssize_t btrfs_sysfs_missing_show(struct kobject *kobj, return snprintf(buf, PAGE_SIZE, "%d\n", val); } -BTRFS_ATTR(devid, missing, btrfs_sysfs_missing_show); +BTRFS_ATTR(devid, missing, btrfs_devinfo_missing_show); static ssize_t btrfs_devinfo_replace_target_show(struct kobject *kobj, struct kobj_attribute *a, -- cgit v1.2.3 From abdd9feb45ed7e9cc55b60a58deba9ca78d46352 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Sun, 22 Mar 2020 10:09:11 +0100 Subject: btrfs: sysfs: Use scnprintf() instead of snprintf() snprintf() is a hard-to-use function, and it's especially difficult to use it properly for concatenating substrings in a buffer with a limited size. Since snprintf() returns the would-be-output size, not the actual size, the subsequent use of snprintf() may point to the incorrect position easily. Also, returning the value from snprintf() directly to sysfs show function would pass a bogus value that is higher than the actually truncated string. That said, although the current code doesn't actually overflow the buffer with PAGE_SIZE, it's a usage that shouldn't be done. Or it's worse; this gives a wrong confidence as if it were doing safe operations. This patch replaces such snprintf() calls with a safer version, scnprintf(). It returns the actual output size, hence it's more intuitive and the code does what's expected. Signed-off-by: Takashi Iwai Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/sysfs.c | 57 ++++++++++++++++++++++++++++---------------------------- 1 file changed, 28 insertions(+), 29 deletions(-) (limited to 'fs/btrfs/sysfs.c') diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c index 93cf76118a04..a39bff64ff24 100644 --- a/fs/btrfs/sysfs.c +++ b/fs/btrfs/sysfs.c @@ -155,7 +155,7 @@ static ssize_t btrfs_feature_attr_show(struct kobject *kobj, } else val = can_modify_feature(fa); - return snprintf(buf, PAGE_SIZE, "%d\n", val); + return scnprintf(buf, PAGE_SIZE, "%d\n", val); } static ssize_t btrfs_feature_attr_store(struct kobject *kobj, @@ -295,7 +295,7 @@ static const struct attribute_group btrfs_feature_attr_group = { static ssize_t rmdir_subvol_show(struct kobject *kobj, struct kobj_attribute *ka, char *buf) { - return snprintf(buf, PAGE_SIZE, "0\n"); + return scnprintf(buf, PAGE_SIZE, "0\n"); } BTRFS_ATTR(static_feature, rmdir_subvol, rmdir_subvol_show); @@ -310,12 +310,12 @@ static ssize_t supported_checksums_show(struct kobject *kobj, * This "trick" only works as long as 'enum btrfs_csum_type' has * no holes in it */ - ret += snprintf(buf + ret, PAGE_SIZE - ret, "%s%s", + ret += scnprintf(buf + ret, PAGE_SIZE - ret, "%s%s", (i == 0 ? "" : " "), btrfs_super_csum_name(i)); } - ret += snprintf(buf + ret, PAGE_SIZE - ret, "\n"); + ret += scnprintf(buf + ret, PAGE_SIZE - ret, "\n"); return ret; } BTRFS_ATTR(static_feature, supported_checksums, supported_checksums_show); @@ -350,7 +350,7 @@ static ssize_t btrfs_discardable_bytes_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = discard_to_fs_info(kobj); - return snprintf(buf, PAGE_SIZE, "%lld\n", + return scnprintf(buf, PAGE_SIZE, "%lld\n", atomic64_read(&fs_info->discard_ctl.discardable_bytes)); } BTRFS_ATTR(discard, discardable_bytes, btrfs_discardable_bytes_show); @@ -361,7 +361,7 @@ static ssize_t btrfs_discardable_extents_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = discard_to_fs_info(kobj); - return snprintf(buf, PAGE_SIZE, "%d\n", + return scnprintf(buf, PAGE_SIZE, "%d\n", atomic_read(&fs_info->discard_ctl.discardable_extents)); } BTRFS_ATTR(discard, discardable_extents, btrfs_discardable_extents_show); @@ -372,7 +372,7 @@ static ssize_t btrfs_discard_bitmap_bytes_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = discard_to_fs_info(kobj); - return snprintf(buf, PAGE_SIZE, "%lld\n", + return scnprintf(buf, PAGE_SIZE, "%lld\n", fs_info->discard_ctl.discard_bitmap_bytes); } BTRFS_ATTR(discard, discard_bitmap_bytes, btrfs_discard_bitmap_bytes_show); @@ -383,7 +383,7 @@ static ssize_t btrfs_discard_bytes_saved_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = discard_to_fs_info(kobj); - return snprintf(buf, PAGE_SIZE, "%lld\n", + return scnprintf(buf, PAGE_SIZE, "%lld\n", atomic64_read(&fs_info->discard_ctl.discard_bytes_saved)); } BTRFS_ATTR(discard, discard_bytes_saved, btrfs_discard_bytes_saved_show); @@ -394,7 +394,7 @@ static ssize_t btrfs_discard_extent_bytes_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = discard_to_fs_info(kobj); - return snprintf(buf, PAGE_SIZE, "%lld\n", + return scnprintf(buf, PAGE_SIZE, "%lld\n", fs_info->discard_ctl.discard_extent_bytes); } BTRFS_ATTR(discard, discard_extent_bytes, btrfs_discard_extent_bytes_show); @@ -405,7 +405,7 @@ static ssize_t btrfs_discard_iops_limit_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = discard_to_fs_info(kobj); - return snprintf(buf, PAGE_SIZE, "%u\n", + return scnprintf(buf, PAGE_SIZE, "%u\n", READ_ONCE(fs_info->discard_ctl.iops_limit)); } @@ -435,7 +435,7 @@ static ssize_t btrfs_discard_kbps_limit_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = discard_to_fs_info(kobj); - return snprintf(buf, PAGE_SIZE, "%u\n", + return scnprintf(buf, PAGE_SIZE, "%u\n", READ_ONCE(fs_info->discard_ctl.kbps_limit)); } @@ -465,7 +465,7 @@ static ssize_t btrfs_discard_max_discard_size_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = discard_to_fs_info(kobj); - return snprintf(buf, PAGE_SIZE, "%llu\n", + return scnprintf(buf, PAGE_SIZE, "%llu\n", READ_ONCE(fs_info->discard_ctl.max_discard_size)); } @@ -530,7 +530,7 @@ static ssize_t btrfs_show_u64(u64 *value_ptr, spinlock_t *lock, char *buf) val = *value_ptr; if (lock) spin_unlock(lock); - return snprintf(buf, PAGE_SIZE, "%llu\n", val); + return scnprintf(buf, PAGE_SIZE, "%llu\n", val); } static ssize_t global_rsv_size_show(struct kobject *kobj, @@ -576,7 +576,7 @@ static ssize_t raid_bytes_show(struct kobject *kobj, val += block_group->used; } up_read(&sinfo->groups_sem); - return snprintf(buf, PAGE_SIZE, "%llu\n", val); + return scnprintf(buf, PAGE_SIZE, "%llu\n", val); } static struct attribute *raid_attrs[] = { @@ -613,7 +613,7 @@ static ssize_t btrfs_space_info_show_total_bytes_pinned(struct kobject *kobj, { struct btrfs_space_info *sinfo = to_space_info(kobj); s64 val = percpu_counter_sum(&sinfo->total_bytes_pinned); - return snprintf(buf, PAGE_SIZE, "%lld\n", val); + return scnprintf(buf, PAGE_SIZE, "%lld\n", val); } SPACE_INFO_ATTR(flags); @@ -670,7 +670,7 @@ static ssize_t btrfs_label_show(struct kobject *kobj, ssize_t ret; spin_lock(&fs_info->super_lock); - ret = snprintf(buf, PAGE_SIZE, label[0] ? "%s\n" : "%s", label); + ret = scnprintf(buf, PAGE_SIZE, label[0] ? "%s\n" : "%s", label); spin_unlock(&fs_info->super_lock); return ret; @@ -718,7 +718,7 @@ static ssize_t btrfs_nodesize_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = to_fs_info(kobj); - return snprintf(buf, PAGE_SIZE, "%u\n", fs_info->super_copy->nodesize); + return scnprintf(buf, PAGE_SIZE, "%u\n", fs_info->super_copy->nodesize); } BTRFS_ATTR(, nodesize, btrfs_nodesize_show); @@ -728,8 +728,8 @@ static ssize_t btrfs_sectorsize_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = to_fs_info(kobj); - return snprintf(buf, PAGE_SIZE, "%u\n", - fs_info->super_copy->sectorsize); + return scnprintf(buf, PAGE_SIZE, "%u\n", + fs_info->super_copy->sectorsize); } BTRFS_ATTR(, sectorsize, btrfs_sectorsize_show); @@ -739,8 +739,7 @@ static ssize_t btrfs_clone_alignment_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = to_fs_info(kobj); - return snprintf(buf, PAGE_SIZE, "%u\n", - fs_info->super_copy->sectorsize); + return scnprintf(buf, PAGE_SIZE, "%u\n", fs_info->super_copy->sectorsize); } BTRFS_ATTR(, clone_alignment, btrfs_clone_alignment_show); @@ -752,7 +751,7 @@ static ssize_t quota_override_show(struct kobject *kobj, int quota_override; quota_override = test_bit(BTRFS_FS_QUOTA_OVERRIDE, &fs_info->flags); - return snprintf(buf, PAGE_SIZE, "%d\n", quota_override); + return scnprintf(buf, PAGE_SIZE, "%d\n", quota_override); } static ssize_t quota_override_store(struct kobject *kobj, @@ -790,7 +789,7 @@ static ssize_t btrfs_metadata_uuid_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = to_fs_info(kobj); - return snprintf(buf, PAGE_SIZE, "%pU\n", + return scnprintf(buf, PAGE_SIZE, "%pU\n", fs_info->fs_devices->metadata_uuid); } @@ -802,7 +801,7 @@ static ssize_t btrfs_checksum_show(struct kobject *kobj, struct btrfs_fs_info *fs_info = to_fs_info(kobj); u16 csum_type = btrfs_super_csum_type(fs_info->super_copy); - return snprintf(buf, PAGE_SIZE, "%s (%s)\n", + return scnprintf(buf, PAGE_SIZE, "%s (%s)\n", btrfs_super_csum_name(csum_type), crypto_shash_driver_name(fs_info->csum_shash)); } @@ -992,7 +991,7 @@ char *btrfs_printable_features(enum btrfs_feature_set set, u64 flags) continue; name = btrfs_feature_attrs[set][i].kobj_attr.attr.name; - len += snprintf(str + len, bufsize - len, "%s%s", + len += scnprintf(str + len, bufsize - len, "%s%s", len ? "," : "", name); } @@ -1201,7 +1200,7 @@ static ssize_t btrfs_devinfo_in_fs_metadata_show(struct kobject *kobj, val = !!test_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state); - return snprintf(buf, PAGE_SIZE, "%d\n", val); + return scnprintf(buf, PAGE_SIZE, "%d\n", val); } BTRFS_ATTR(devid, in_fs_metadata, btrfs_devinfo_in_fs_metadata_show); @@ -1214,7 +1213,7 @@ static ssize_t btrfs_devinfo_missing_show(struct kobject *kobj, val = !!test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state); - return snprintf(buf, PAGE_SIZE, "%d\n", val); + return scnprintf(buf, PAGE_SIZE, "%d\n", val); } BTRFS_ATTR(devid, missing, btrfs_devinfo_missing_show); @@ -1228,7 +1227,7 @@ static ssize_t btrfs_devinfo_replace_target_show(struct kobject *kobj, val = !!test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state); - return snprintf(buf, PAGE_SIZE, "%d\n", val); + return scnprintf(buf, PAGE_SIZE, "%d\n", val); } BTRFS_ATTR(devid, replace_target, btrfs_devinfo_replace_target_show); @@ -1241,7 +1240,7 @@ static ssize_t btrfs_devinfo_writeable_show(struct kobject *kobj, val = !!test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state); - return snprintf(buf, PAGE_SIZE, "%d\n", val); + return scnprintf(buf, PAGE_SIZE, "%d\n", val); } BTRFS_ATTR(devid, writeable, btrfs_devinfo_writeable_show); -- cgit v1.2.3