Re: [PATCH 4/4] btrfs: sysfs, add devid/dev_state kobject and attribute

David Sterba
2019-12-05T14:21:56Z

On Thu, Dec 05, 2019 at 07:27:06PM +0800, Anand Jain wrote:
> A new sysfs RW attribute

Why RW? Most attributes/files in sysfs are supposed to be read-only, but
you actually define the attribute as read-only with the macro
BTRFS_ATTR.

> UUID/devinfo/<devid>/dev_state
> is added here. The dev_state here reflects the state of the device from
> the kernel parameter btrfs_device::dev_state and this attribute is born
> after the device is mounted and goes along with the dynamic nature of the
> device add and delete. This attribute gets deleted at unmount.
>
> For example:
> pwd
> /sys/fs/btrfs/6e1961f1-5918-4ecc-a22f-948897b409f7/devinfo
> cat 1/dev_state
> IN_FS_METADATA MISSING
> cat 2/dev_state
> WRITABLE IN_FS_METADATA

So the values copy the device state macros, that's probably ok.

> +static ssize_t btrfs_sysfs_dev_state_show(struct kobject *kobj,
> + struct kobj_attribute *a, char *buf)
> +{
> + struct btrfs_device *device = container_of(kobj, struct btrfs_device,
> + devid_kobj);
> +
> + btrfs_dev_state_to_str(device, buf, PAGE_SIZE);

The device access is unprotected, you need at least RCU but that still
does not prevent from the device being freed by deletion. The
device_list_mutex is quite heavy and allowing a DoS by repeatedly
reading the file contents is not something we want to allow.