On 5/20/14, 1:36 AM, Anand Jain wrote: > From: Anand Jain <anand.j...@oracle.com> > > generally if you use > echo "test" > /sys/fs/btrfs/<fsid>/label > it would introduce return char at the end and it can not > be part of the label. The correct command is > echo -n "test" > /sys/fs/btrfs/<fsid>/label > > This patch will check for this user error > > Signed-off-by: Anand Jain <anand.j...@oracle.com> > --- > v2: accepts review comments. Thanks Eric and Roman > > fs/btrfs/sysfs.c | 20 +++++++++++++++++--- > 1 files changed, 17 insertions(+), 3 deletions(-) > > diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c > index c5eb214..ca63fcd 100644 > --- a/fs/btrfs/sysfs.c > +++ b/fs/btrfs/sysfs.c > @@ -373,22 +373,36 @@ static ssize_t btrfs_label_store(struct kobject *kobj, > struct btrfs_trans_handle *trans; > struct btrfs_root *root = fs_info->fs_root; > int ret; > + char *label; > + char *pos; > > - if (len >= BTRFS_LABEL_SIZE) { > + label = kzalloc(len, GFP_NOFS); > + if (!label) > + return -ENOMEM; > + > + memcpy(label, buf, len);
+ strlcpy(label, buf, len); would ensure that the resulting string is null-terminated... I don't know if "buf" comes in 0-terminated or not, or if len includes \0. *shrug* these are strings after all... > + if ((pos = strchr(label, '\n'))) > + *pos = '\0'; label = strstrip(label); might be simpler/better? (this would strip all leading & trailing whitespace, I presume that'd be desirable, but then maybe someone really does want " mylabel \t " ?) > + > + if (strlen(label) >= BTRFS_LABEL_SIZE) { hm, strlen doesn't include the \0 right? so if we had 256 chars + \0, this would pass, and the subsequent strcpy will copy 257 bytes into a 256-byte buffer, right? (I'm terrible at string handling in C, so I might be wrong... you all can point and laugh if I am) > pr_err("BTRFS: unable to set label with more than %d bytes\n", > BTRFS_LABEL_SIZE - 1); > + kfree(label); > return -EINVAL; > } > > trans = btrfs_start_transaction(root, 0); > - if (IS_ERR(trans)) > + if (IS_ERR(trans)) { > + kfree(label); > return PTR_ERR(trans); > + } > > spin_lock(&root->fs_info->super_lock); > - strcpy(fs_info->super_copy->label, buf); > + strcpy(fs_info->super_copy->label, label); > spin_unlock(&root->fs_info->super_lock); > ret = btrfs_commit_transaction(trans, root); > > + kfree(label); after the 3rd kfree() maybe an out: target would be better.... (Random aside: why does btrfs support online fs relabeling, anyway?) -Eric > if (!ret) > return len; > > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html