| Message ID | f7692e58a8e6a7bd28160adfc664e9e8ccfa6fcc.1461681045.git.dsterba@suse.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Tue, Apr 26, 2016 at 3:32 PM, David Sterba <dsterba@suse.com> wrote: > If the label setting ioctl races with sysfs label handler, we could get > mixed result in the output, part old part new. We should either get the > old or new label. The chances to hit this race are low. > > Signed-off-by: David Sterba <dsterba@suse.com> > --- > fs/btrfs/sysfs.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c > index 3d14618ce54b..7b0da1dcb6df 100644 > --- a/fs/btrfs/sysfs.c > +++ b/fs/btrfs/sysfs.c > @@ -367,7 +367,12 @@ static ssize_t btrfs_label_show(struct kobject *kobj, > { > struct btrfs_fs_info *fs_info = to_fs_info(kobj); > char *label = fs_info->super_copy->label; > - return snprintf(buf, PAGE_SIZE, label[0] ? "%s\n" : "%s", label); > + > + spin_lock(&fs_info->super_lock); > + snprintf(buf, PAGE_SIZE, label[0] ? "%s\n" : "%s", label); > + spin_unlock(&fs_info->super_lock); > + > + return buf; We should return a ssize_t value, not a char *. I.e. return what snprintf returns. This should make gcc emit a warning. > } > > static ssize_t btrfs_label_store(struct kobject *kobj, > -- > 2.7.1 > > -- > 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 Tue, Apr 26, 2016 at 04:52:09PM +0100, Filipe Manana wrote: > On Tue, Apr 26, 2016 at 3:32 PM, David Sterba <dsterba@suse.com> wrote: > > If the label setting ioctl races with sysfs label handler, we could get > > mixed result in the output, part old part new. We should either get the > > old or new label. The chances to hit this race are low. > > > > Signed-off-by: David Sterba <dsterba@suse.com> > > --- > > fs/btrfs/sysfs.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c > > index 3d14618ce54b..7b0da1dcb6df 100644 > > --- a/fs/btrfs/sysfs.c > > +++ b/fs/btrfs/sysfs.c > > @@ -367,7 +367,12 @@ static ssize_t btrfs_label_show(struct kobject *kobj, > > { > > struct btrfs_fs_info *fs_info = to_fs_info(kobj); > > char *label = fs_info->super_copy->label; > > - return snprintf(buf, PAGE_SIZE, label[0] ? "%s\n" : "%s", label); > > + > > + spin_lock(&fs_info->super_lock); > > + snprintf(buf, PAGE_SIZE, label[0] ? "%s\n" : "%s", label); > > + spin_unlock(&fs_info->super_lock); > > + > > + return buf; > > We should return a ssize_t value, not a char *. I.e. return what > snprintf returns. This should make gcc emit a warning. Indeed the warning was there and I overlooked it, last minute patches before leaving. Thanks for catching it. -- 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/sysfs.c b/fs/btrfs/sysfs.c index 3d14618ce54b..7b0da1dcb6df 100644 --- a/fs/btrfs/sysfs.c +++ b/fs/btrfs/sysfs.c @@ -367,7 +367,12 @@ static ssize_t btrfs_label_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = to_fs_info(kobj); char *label = fs_info->super_copy->label; - return snprintf(buf, PAGE_SIZE, label[0] ? "%s\n" : "%s", label); + + spin_lock(&fs_info->super_lock); + snprintf(buf, PAGE_SIZE, label[0] ? "%s\n" : "%s", label); + spin_unlock(&fs_info->super_lock); + + return buf; } static ssize_t btrfs_label_store(struct kobject *kobj,
If the label setting ioctl races with sysfs label handler, we could get mixed result in the output, part old part new. We should either get the old or new label. The chances to hit this race are low. Signed-off-by: David Sterba <dsterba@suse.com> --- fs/btrfs/sysfs.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)