linux-kernel - Re: [PATCH] btrfs: Accept and ignore compression level for lzo

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPjX3FeOEg+QhkwKWe+qDH876bp6-t1GFO0sce7a6bmhM7umpw@mail.gmail.com>
Date: Mon, 25 Aug 2025 10:51:24 +0200
From: Daniel Vacek <neelx@...e.com>
To: Calvin Owens <calvin@...nvd.org>
Cc: Qu Wenruo <quwenruo.btrfs@....com>, Sun YangKai <sunk67188@...il.com>, clm@...com, 
	dsterba@...e.com, josef@...icpanda.com, linux-btrfs@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] btrfs: Accept and ignore compression level for lzo

On Sun, 24 Aug 2025 at 17:58, Calvin Owens <calvin@...nvd.org> wrote:
> From: Calvin Owens <calvin@...nvd.org>
> Subject: [PATCH v3] btrfs: Accept and ignore compression level for lzo
>
> The compression level is meaningless for lzo, but before commit
> 3f093ccb95f30 ("btrfs: harden parsing of compression mount options"),
> it was silently ignored if passed.
>
> After that commit, passing a level with lzo fails to mount:
>
>     BTRFS error: unrecognized compression value lzo:1
>
> It seems reasonable for users to expect that lzo would permit a numeric
> level option, as all the other algos do, even though the kernel's
> implementation of LZO currently only supports a single level. Because it
> has always worked to pass a level, it seems likely to me that users in
> the real world are relying on doing so.
>
> This patch restores the old behavior, giving "lzo:N" the same semantics
> as all of the other compression algos.
>
> To be clear, silly variants like "lzo:one", "lzo:the_first_option", or
> "lzo:armageddon" also used to work. This isn't meant to suggest that
> any possible mis-interpretation of mount options that once worked must
> continue to work forever. This is an exceptional case where it makes
> sense to preserve compatibility, both because the mis-interpretation is
> reasonable, and because nothing tangible is sacrificed.
>
> Fixes: 3f093ccb95f30 ("btrfs: harden parsing of compression mount options")
> Signed-off-by: Calvin Owens <calvin@...nvd.org>
> ---
>  fs/btrfs/super.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)

v3 looks good to me. The original hardening was meant to gate complete
nonsense like "compress=lzoutput", etc...

Reviewed-by: Daniel Vacek <neelx@...e.com>

Thank you.

> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index a262b494a89f..18eb00b3639b 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -299,9 +299,12 @@ 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 (btrfs_match_compress_type(string, "lzo", false)) {
> +       } else if (btrfs_match_compress_type(string, "lzo", true)) {
>                 ctx->compress_type = BTRFS_COMPRESS_LZO;
> -               ctx->compress_level = 0;
> +               ctx->compress_level = btrfs_compress_str2level(BTRFS_COMPRESS_LZO,
> +                                                              string + 3);
> +               if (string[3] == ':' && string[4])
> +                       btrfs_warn(NULL, "Compression level ignored for LZO");
>                 btrfs_set_opt(ctx->mount_opt, COMPRESS);
>                 btrfs_clear_opt(ctx->mount_opt, NODATACOW);
>                 btrfs_clear_opt(ctx->mount_opt, NODATASUM);
> --
> 2.49.1
>