[PATCH v3 2/2] btrfs: harden parsing of compress mount options

Daniel Vacek posted 2 patches 6 months, 2 weeks ago
[PATCH v3 2/2] btrfs: harden parsing of compress mount options
Posted by Daniel Vacek 6 months, 2 weeks ago
Btrfs happily but incorrectly accepts the `-o compress=zlib+foo` and similar
options with any random suffix.

Fix that by explicitly checking the end of the strings.

Signed-off-by: Daniel Vacek <neelx@suse.com>
---
v3 changes: Split into two patches to ease backporting,
            no functional changes.

 fs/btrfs/super.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 6291ab45ab2a5..4510c5f7a785e 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -270,9 +270,20 @@ static inline blk_mode_t btrfs_open_mode(struct fs_context *fc)
 	return sb_open_mode(fc->sb_flags) & ~BLK_OPEN_RESTRICT_WRITES;
 }
 
+static bool btrfs_match_compress_type(char *string, char *type, bool may_have_level)
+{
+	int len = strlen(type);
+
+	return strncmp(string, type, len) == 0 &&
+		((may_have_level && string[len] == ':') ||
+				    string[len] == '\0');
+}
+
 static int btrfs_parse_compress(struct btrfs_fs_context *ctx,
 				struct fs_parameter *param, int opt)
 {
+	char *string = param->string;
+
 	/*
 	 * Provide the same semantics as older kernels that don't use fs
 	 * context, specifying the "compress" option clears
@@ -288,36 +299,37 @@ static int btrfs_parse_compress(struct btrfs_fs_context *ctx,
 		btrfs_set_opt(ctx->mount_opt, COMPRESS);
 		btrfs_clear_opt(ctx->mount_opt, NODATACOW);
 		btrfs_clear_opt(ctx->mount_opt, NODATASUM);
-	} else if (strncmp(param->string, "zlib", 4) == 0) {
+	} else if (btrfs_match_compress_type(string, "zlib", true)) {
 		ctx->compress_type = BTRFS_COMPRESS_ZLIB;
 		ctx->compress_level =
 			btrfs_compress_str2level(BTRFS_COMPRESS_ZLIB,
-						 param->string + 4);
+						 string + 4);
 		btrfs_set_opt(ctx->mount_opt, COMPRESS);
 		btrfs_clear_opt(ctx->mount_opt, NODATACOW);
 		btrfs_clear_opt(ctx->mount_opt, NODATASUM);
-	} else if (strncmp(param->string, "lzo", 3) == 0) {
+	} else if (btrfs_match_compress_type(string, "lzo", false)) {
 		ctx->compress_type = BTRFS_COMPRESS_LZO;
 		ctx->compress_level = 0;
 		btrfs_set_opt(ctx->mount_opt, COMPRESS);
 		btrfs_clear_opt(ctx->mount_opt, NODATACOW);
 		btrfs_clear_opt(ctx->mount_opt, NODATASUM);
-	} else if (strncmp(param->string, "zstd", 4) == 0) {
+	} else if (btrfs_match_compress_type(string, "zstd", true)) {
 		ctx->compress_type = BTRFS_COMPRESS_ZSTD;
 		ctx->compress_level =
 			btrfs_compress_str2level(BTRFS_COMPRESS_ZSTD,
-						 param->string + 4);
+						 string + 4);
 		btrfs_set_opt(ctx->mount_opt, COMPRESS);
 		btrfs_clear_opt(ctx->mount_opt, NODATACOW);
 		btrfs_clear_opt(ctx->mount_opt, NODATASUM);
-	} else if (strncmp(param->string, "no", 2) == 0) {
+	} else if (btrfs_match_compress_type(string, "no", false) ||
+		   btrfs_match_compress_type(string, "none", false)) {
 		ctx->compress_level = 0;
 		ctx->compress_type = 0;
 		btrfs_clear_opt(ctx->mount_opt, COMPRESS);
 		btrfs_clear_opt(ctx->mount_opt, FORCE_COMPRESS);
 	} else {
 		btrfs_err(NULL, "unrecognized compression value %s",
-			  param->string);
+			  string);
 		return -EINVAL;
 	}
 	return 0;
-- 
2.47.2
Re: [PATCH v3 2/2] btrfs: harden parsing of compress mount options
Posted by David Sterba 6 months, 2 weeks ago
On Mon, Jun 02, 2025 at 05:53:19PM +0200, Daniel Vacek wrote:
> Btrfs happily but incorrectly accepts the `-o compress=zlib+foo` and similar
> options with any random suffix.
> 
> Fix that by explicitly checking the end of the strings.
> 
> Signed-off-by: Daniel Vacek <neelx@suse.com>
> ---
> v3 changes: Split into two patches to ease backporting,
>             no functional changes.
> 
>  fs/btrfs/super.c | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 6291ab45ab2a5..4510c5f7a785e 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -270,9 +270,20 @@ static inline blk_mode_t btrfs_open_mode(struct fs_context *fc)
>  	return sb_open_mode(fc->sb_flags) & ~BLK_OPEN_RESTRICT_WRITES;
>  }
>  
> +static bool btrfs_match_compress_type(char *string, char *type, bool may_have_level)

const also here, string, type

> +{
> +	int len = strlen(type);
> +
> +	return strncmp(string, type, len) == 0 &&
> +		((may_have_level && string[len] == ':') ||
> +				    string[len] == '\0');
> +}
> +
>  static int btrfs_parse_compress(struct btrfs_fs_context *ctx,
>  				struct fs_parameter *param, int opt)
>  {
> +	char *string = param->string;

and here

> +
>  	/*
>  	 * Provide the same semantics as older kernels that don't use fs
>  	 * context, specifying the "compress" option clears
Re: [PATCH v3 2/2] btrfs: harden parsing of compress mount options
Posted by Daniel Vacek 6 months, 1 week ago
On Mon, 2 Jun 2025 at 19:29, David Sterba <dsterba@suse.cz> wrote:
>
> On Mon, Jun 02, 2025 at 05:53:19PM +0200, Daniel Vacek wrote:
> > Btrfs happily but incorrectly accepts the `-o compress=zlib+foo` and similar
> > options with any random suffix.
> >
> > Fix that by explicitly checking the end of the strings.
> >
> > Signed-off-by: Daniel Vacek <neelx@suse.com>
> > ---
> > v3 changes: Split into two patches to ease backporting,
> >             no functional changes.
> >
> >  fs/btrfs/super.c | 26 +++++++++++++++++++-------
> >  1 file changed, 19 insertions(+), 7 deletions(-)
> >
> > diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> > index 6291ab45ab2a5..4510c5f7a785e 100644
> > --- a/fs/btrfs/super.c
> > +++ b/fs/btrfs/super.c
> > @@ -270,9 +270,20 @@ static inline blk_mode_t btrfs_open_mode(struct fs_context *fc)
> >       return sb_open_mode(fc->sb_flags) & ~BLK_OPEN_RESTRICT_WRITES;
> >  }
> >
> > +static bool btrfs_match_compress_type(char *string, char *type, bool may_have_level)
>
> const also here, string, type
>
> > +{
> > +     int len = strlen(type);
> > +
> > +     return strncmp(string, type, len) == 0 &&
> > +             ((may_have_level && string[len] == ':') ||
> > +                                 string[len] == '\0');
> > +}
> > +
> >  static int btrfs_parse_compress(struct btrfs_fs_context *ctx,
> >                               struct fs_parameter *param, int opt)
> >  {
> > +     char *string = param->string;
>
> and here

Can be done at merge time. Or do you want a re-send?


> > +
> >       /*
> >        * Provide the same semantics as older kernels that don't use fs
> >        * context, specifying the "compress" option clears
Re: [PATCH v3 2/2] btrfs: harden parsing of compress mount options
Posted by David Sterba 6 months, 1 week ago
On Thu, Jun 12, 2025 at 03:29:21PM +0200, Daniel Vacek wrote:
> On Mon, 2 Jun 2025 at 19:29, David Sterba <dsterba@suse.cz> wrote:
> >
> > On Mon, Jun 02, 2025 at 05:53:19PM +0200, Daniel Vacek wrote:
> > > Btrfs happily but incorrectly accepts the `-o compress=zlib+foo` and similar
> > > options with any random suffix.
> > >
> > > Fix that by explicitly checking the end of the strings.
> > >
> > > Signed-off-by: Daniel Vacek <neelx@suse.com>
> > > ---
> > > v3 changes: Split into two patches to ease backporting,
> > >             no functional changes.
> > >
> > >  fs/btrfs/super.c | 26 +++++++++++++++++++-------
> > >  1 file changed, 19 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> > > index 6291ab45ab2a5..4510c5f7a785e 100644
> > > --- a/fs/btrfs/super.c
> > > +++ b/fs/btrfs/super.c
> > > @@ -270,9 +270,20 @@ static inline blk_mode_t btrfs_open_mode(struct fs_context *fc)
> > >       return sb_open_mode(fc->sb_flags) & ~BLK_OPEN_RESTRICT_WRITES;
> > >  }
> > >
> > > +static bool btrfs_match_compress_type(char *string, char *type, bool may_have_level)
> >
> > const also here, string, type
> >
> > > +{
> > > +     int len = strlen(type);
> > > +
> > > +     return strncmp(string, type, len) == 0 &&
> > > +             ((may_have_level && string[len] == ':') ||
> > > +                                 string[len] == '\0');
> > > +}
> > > +
> > >  static int btrfs_parse_compress(struct btrfs_fs_context *ctx,
> > >                               struct fs_parameter *param, int opt)
> > >  {
> > > +     char *string = param->string;
> >
> > and here
> 
> Can be done at merge time. Or do you want a re-send?

No resend needed, I updated the patch in for-next. This was to let you
know so that I don't need to fix it in future patches.