* [PATCH 0/3] Minor updates to sysfs
@ 2016-04-26 14:32 David Sterba
2016-04-26 14:32 ` [PATCH 1/3] btrfs: add read-only check to sysfs handler of features David Sterba
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: David Sterba @ 2016-04-26 14:32 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
Hi,
a less-than-handful set of fixes to sysfs, two sanity checks and one additional
locking preventing a pretty rare race.
David Sterba (3):
btrfs: add read-only check to sysfs handler of features
btrfs: add check to sysfs handler of label
btrfs: sysfs: protect reading label by lock
fs/btrfs/sysfs.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
--
2.7.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/3] btrfs: add read-only check to sysfs handler of features 2016-04-26 14:32 [PATCH 0/3] Minor updates to sysfs David Sterba @ 2016-04-26 14:32 ` David Sterba 2016-04-26 14:32 ` [PATCH 2/3] btrfs: add check to sysfs handler of label David Sterba 2016-04-26 14:32 ` [PATCH 3/3] btrfs: sysfs: protect reading label by lock David Sterba 2 siblings, 0 replies; 6+ messages in thread From: David Sterba @ 2016-04-26 14:32 UTC (permalink / raw) To: linux-btrfs; +Cc: David Sterba From: David Sterba <dsterba@suse.cz> We don't want to trigger the change on a read-only filesystem, similar to what the label handler does. Signed-off-by: David Sterba <dsterba@suse.cz> --- fs/btrfs/sysfs.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c index 539e7b5e3f86..6a6bb600b1ff 100644 --- a/fs/btrfs/sysfs.c +++ b/fs/btrfs/sysfs.c @@ -120,6 +120,9 @@ static ssize_t btrfs_feature_attr_store(struct kobject *kobj, if (!fs_info) return -EPERM; + if (fs_info->sb->s_flags & MS_RDONLY) + return -EROFS; + ret = kstrtoul(skip_spaces(buf), 0, &val); if (ret) return ret; -- 2.7.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] btrfs: add check to sysfs handler of label 2016-04-26 14:32 [PATCH 0/3] Minor updates to sysfs David Sterba 2016-04-26 14:32 ` [PATCH 1/3] btrfs: add read-only check to sysfs handler of features David Sterba @ 2016-04-26 14:32 ` David Sterba 2016-04-26 14:32 ` [PATCH 3/3] btrfs: sysfs: protect reading label by lock David Sterba 2 siblings, 0 replies; 6+ messages in thread From: David Sterba @ 2016-04-26 14:32 UTC (permalink / raw) To: linux-btrfs; +Cc: David Sterba Add a sanity check for the fs_info as we will dereference it, similar to what the 'store features' handler does. Signed-off-by: David Sterba <dsterba@suse.com> --- fs/btrfs/sysfs.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c index 6a6bb600b1ff..3d14618ce54b 100644 --- a/fs/btrfs/sysfs.c +++ b/fs/btrfs/sysfs.c @@ -377,6 +377,9 @@ static ssize_t btrfs_label_store(struct kobject *kobj, struct btrfs_fs_info *fs_info = to_fs_info(kobj); size_t p_len; + if (!fs_info) + return -EPERM; + if (fs_info->sb->s_flags & MS_RDONLY) return -EROFS; -- 2.7.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] btrfs: sysfs: protect reading label by lock 2016-04-26 14:32 [PATCH 0/3] Minor updates to sysfs David Sterba 2016-04-26 14:32 ` [PATCH 1/3] btrfs: add read-only check to sysfs handler of features David Sterba 2016-04-26 14:32 ` [PATCH 2/3] btrfs: add check to sysfs handler of label David Sterba @ 2016-04-26 14:32 ` David Sterba 2016-04-26 15:52 ` Filipe Manana 2 siblings, 1 reply; 6+ messages in thread From: David Sterba @ 2016-04-26 14:32 UTC (permalink / raw) To: linux-btrfs; +Cc: David Sterba 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; } static ssize_t btrfs_label_store(struct kobject *kobj, -- 2.7.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] btrfs: sysfs: protect reading label by lock 2016-04-26 14:32 ` [PATCH 3/3] btrfs: sysfs: protect reading label by lock David Sterba @ 2016-04-26 15:52 ` Filipe Manana 2016-04-26 21:44 ` David Sterba 0 siblings, 1 reply; 6+ messages in thread From: Filipe Manana @ 2016-04-26 15:52 UTC (permalink / raw) To: David Sterba; +Cc: linux-btrfs@vger.kernel.org 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 -- Filipe David Manana, "Reasonable men adapt themselves to the world. Unreasonable men adapt the world to themselves. That's why all progress depends on unreasonable men." ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] btrfs: sysfs: protect reading label by lock 2016-04-26 15:52 ` Filipe Manana @ 2016-04-26 21:44 ` David Sterba 0 siblings, 0 replies; 6+ messages in thread From: David Sterba @ 2016-04-26 21:44 UTC (permalink / raw) To: Filipe Manana; +Cc: linux-btrfs@vger.kernel.org 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-04-26 21:44 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-04-26 14:32 [PATCH 0/3] Minor updates to sysfs David Sterba 2016-04-26 14:32 ` [PATCH 1/3] btrfs: add read-only check to sysfs handler of features David Sterba 2016-04-26 14:32 ` [PATCH 2/3] btrfs: add check to sysfs handler of label David Sterba 2016-04-26 14:32 ` [PATCH 3/3] btrfs: sysfs: protect reading label by lock David Sterba 2016-04-26 15:52 ` Filipe Manana 2016-04-26 21:44 ` David Sterba
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).