bcachefs: Better superblock opt validation

This moves validation of superblock options to bch2_sb_validate(), so
they'll be checked in the write path as well.

Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com>

fs/bcachefs/opts.c

@@ -224,42 +224,43 @@ static int bch2_mount_opt_lookup(const char *name)
 	return bch2_opt_lookup(name);
 }
 
-static int bch2_opt_validate(const struct bch_option *opt, const char *msg, u64 v)
+int bch2_opt_validate(const struct bch_option *opt, u64 v, struct printbuf *err)
 {
 	if (v < opt->min) {
-		if (msg)
+		if (err)
-			pr_err("invalid %s%s: too small (min %llu)",
+			pr_buf(err, "%s: too small (min %llu)",
-			       msg, opt->attr.name, opt->min);
+			       opt->attr.name, opt->min);
 		return -ERANGE;
 	}
 
 	if (opt->max && v >= opt->max) {
-		if (msg)
+		if (err)
-			pr_err("invalid %s%s: too big (max %llu)",
+			pr_buf(err, "%s: too big (max %llu)",
-			       msg, opt->attr.name, opt->max);
+			       opt->attr.name, opt->max);
 		return -ERANGE;
 	}
 
 	if ((opt->flags & OPT_SB_FIELD_SECTORS) && (v & 511)) {
-		if (msg)
+		if (err)
-			pr_err("invalid %s %s: not a multiple of 512",
+			pr_buf(err, "%s: not a multiple of 512",
-			       msg, opt->attr.name);
+			       opt->attr.name);
 		return -EINVAL;
 	}
 
 	if ((opt->flags & OPT_MUST_BE_POW_2) && !is_power_of_2(v)) {
-		if (msg)
+		if (err)
-			pr_err("invalid %s%s: must be a power of two",
+			pr_buf(err, "%s: must be a power of two",
-			       msg, opt->attr.name);
+			       opt->attr.name);
 		return -EINVAL;
 	}
 
 	return 0;
 }
 
-int bch2_opt_parse(struct bch_fs *c, const char *msg,
+int bch2_opt_parse(struct bch_fs *c,
 		   const struct bch_option *opt,
-		   const char *val, u64 *res)
+		   const char *val, u64 *res,
+		   struct printbuf *err)
 {
 	ssize_t ret;
 
@@ -292,7 +293,7 @@ int bch2_opt_parse(struct bch_fs *c, const char *msg,
 			return ret;
 	}
 
-	return bch2_opt_validate(opt, msg, *res);
+	return bch2_opt_validate(opt, *res, err);
 }
 
 void bch2_opt_to_text(struct printbuf *out,
@@ -372,6 +373,7 @@ int bch2_parse_mount_opts(struct bch_fs *c, struct bch_opts *opts,
 	char *copied_opts, *copied_opts_start;
 	char *opt, *name, *val;
 	int ret, id;
+	struct printbuf err = PRINTBUF;
 	u64 v;
 
 	if (!options)
@@ -391,8 +393,7 @@ int bch2_parse_mount_opts(struct bch_fs *c, struct bch_opts *opts,
 			if (id < 0)
 				goto bad_opt;
 
-			ret = bch2_opt_parse(c, "mount option ",
+			ret = bch2_opt_parse(c, &bch2_opt_table[id], val, &v, &err);
-					     &bch2_opt_table[id], val, &v);
 			if (ret < 0)
 				goto bad_val;
 		} else {
@@ -435,7 +436,7 @@ bad_opt:
 	ret = -1;
 	goto out;
 bad_val:
-	pr_err("Invalid value %s for mount option %s", val, name);
+	pr_err("Invalid mount option %s", err.buf);
 	ret = -1;
 	goto out;
 no_val:
@@ -444,6 +445,7 @@ no_val:
 	goto out;
 out:
 	kfree(copied_opts_start);
+	printbuf_exit(&err);
 	return ret;
 }
 
@@ -470,22 +472,14 @@ u64 bch2_opt_from_sb(struct bch_sb *sb, enum bch_opt_id id)
 int bch2_opts_from_sb(struct bch_opts *opts, struct bch_sb *sb)
 {
 	unsigned id;
-	int ret;
 
 	for (id = 0; id < bch2_opts_nr; id++) {
 		const struct bch_option *opt = bch2_opt_table + id;
-		u64 v;
 
 		if (opt->get_sb == BCH2_NO_SB_OPT)
 			continue;
 
-		v = bch2_opt_from_sb(sb, id);
+		bch2_opt_set_by_id(opts, id, bch2_opt_from_sb(sb, id));
-
-		ret = bch2_opt_validate(opt, "superblock option ", v);
-		if (ret)
-			return ret;
-
-		bch2_opt_set_by_id(opts, id, v);
 	}
 
 	return 0;

fs/bcachefs/opts.h

@@ -489,8 +489,9 @@ void __bch2_opt_set_sb(struct bch_sb *, const struct bch_option *, u64);
 void bch2_opt_set_sb(struct bch_fs *, const struct bch_option *, u64);
 
 int bch2_opt_lookup(const char *);
-int bch2_opt_parse(struct bch_fs *, const char *, const struct bch_option *,
+int bch2_opt_validate(const struct bch_option *, u64, struct printbuf *);
-		   const char *, u64 *);
+int bch2_opt_parse(struct bch_fs *, const struct bch_option *,
+		   const char *, u64 *, struct printbuf *);
 
 #define OPT_SHOW_FULL_LIST	(1 << 0)
 #define OPT_SHOW_MOUNT_STYLE	(1 << 1)

fs/bcachefs/super-io.c

@@ -258,6 +258,7 @@ static int bch2_sb_validate(struct bch_sb_handle *disk_sb, struct printbuf *out)
 	struct bch_sb *sb = disk_sb->sb;
 	struct bch_sb_field *f;
 	struct bch_sb_field_members *mi;
+	enum bch_opt_id opt_id;
 	u32 version, version_min;
 	u16 block_size;
 	int ret;
@@ -329,6 +330,21 @@ static int bch2_sb_validate(struct bch_sb_handle *disk_sb, struct printbuf *out)
 		return -EINVAL;
 	}
 
+	for (opt_id = 0; opt_id < bch2_opts_nr; opt_id++) {
+		const struct bch_option *opt = bch2_opt_table + opt_id;
+
+		if (opt->get_sb != BCH2_NO_SB_OPT) {
+			u64 v = bch2_opt_from_sb(sb, opt_id);
+
+			pr_buf(out, "Invalid option ");
+			ret = bch2_opt_validate(opt, v, out);
+			if (ret)
+				return ret;
+
+			printbuf_reset(out);
+		}
+	}
+
 	/* validate layout */
 	ret = validate_sb_layout(&sb->layout, out);
 	if (ret)

fs/bcachefs/sysfs.c

@@ -624,7 +624,7 @@ STORE(bch2_fs_opts_dir)
 		goto err;
 	}
 
-	ret = bch2_opt_parse(c, NULL, opt, strim(tmp), &v);
+	ret = bch2_opt_parse(c, opt, strim(tmp), &v, NULL);
 	kfree(tmp);
 
 	if (ret < 0)

fs/bcachefs/xattr.c

@@ -525,7 +525,7 @@ static int bch2_xattr_bcachefs_set(const struct xattr_handler *handler,
 		memcpy(buf, value, size);
 		buf[size] = '\0';
 
-		ret = bch2_opt_parse(c, NULL, opt, buf, &v);
+		ret = bch2_opt_parse(c, opt, buf, &v, NULL);
 		kfree(buf);
 
 		if (ret < 0)