| Message ID | 433d5c3c5a47154a8bead243a9f6e0c8b3b7ad7d.1503470354.git.osandov@fb.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Aug 22, 2017 at 11:46:03PM -0700, Omar Sandoval wrote: > From: Omar Sandoval <osandov@fb.com> > > A naked read of the value of an RCU pointer isn't safe. Put the whole > access in an RCU critical section, not just the pointer dereference. In this case it is safe, as the device will not go away (and potentially free dev->name) because it's under the device_list_mutex. The locking around devices and related structures is not exactly straightforward, but here I don't think we need to stick to the RCU pattern if the protection is guaranteed by other means. This applies to uuid_mutex and device_list_mutex. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 06, 2017 at 05:37:28PM +0200, David Sterba wrote: > On Tue, Aug 22, 2017 at 11:46:03PM -0700, Omar Sandoval wrote: > > From: Omar Sandoval <osandov@fb.com> > > > > A naked read of the value of an RCU pointer isn't safe. Put the whole > > access in an RCU critical section, not just the pointer dereference. > > In this case it is safe, as the device will not go away (and potentially > free dev->name) because it's under the device_list_mutex. > > The locking around devices and related structures is not exactly > straightforward, but here I don't think we need to stick to the RCU > pattern if the protection is guaranteed by other means. This applies to > uuid_mutex and device_list_mutex. You're right, it's a little confusing because uuid_mutex protects device->name, so under device_list_mutex, device->name might change but will never become NULL. We can just drop this one, thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index befeacd0e847..cf71d0304671 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -2823,6 +2823,7 @@ static long btrfs_ioctl_dev_info(struct btrfs_fs_info *fs_info, struct btrfs_fs_devices *fs_devices = fs_info->fs_devices; int ret = 0; char *s_uuid = NULL; + struct rcu_string *name; di_args = memdup_user(arg, sizeof(*di_args)); if (IS_ERR(di_args)) @@ -2843,17 +2844,16 @@ static long btrfs_ioctl_dev_info(struct btrfs_fs_info *fs_info, di_args->bytes_used = btrfs_device_get_bytes_used(dev); di_args->total_bytes = btrfs_device_get_total_bytes(dev); memcpy(di_args->uuid, dev->uuid, sizeof(di_args->uuid)); - if (dev->name) { - struct rcu_string *name; - rcu_read_lock(); - name = rcu_dereference(dev->name); + rcu_read_lock(); + name = rcu_dereference(dev->name); + if (name) { strncpy(di_args->path, name->str, sizeof(di_args->path)); - rcu_read_unlock(); di_args->path[sizeof(di_args->path) - 1] = 0; } else { di_args->path[0] = '\0'; } + rcu_read_unlock(); out: mutex_unlock(&fs_devices->device_list_mutex);