linux-kernel - Re: [PATCH] btrfs/defrag: implement compression levels

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: <20250305080150.GB5777@suse.cz>
Date: Wed, 5 Mar 2025 09:01:50 +0100
From: David Sterba <dsterba@...e.cz>
To: Qu Wenruo <wqu@...e.com>
Cc: Daniel Vacek <neelx@...e.com>, Qu Wenruo <quwenruo.btrfs@....com>,
	Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>,
	David Sterba <dsterba@...e.com>, linux-btrfs@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] btrfs/defrag: implement compression levels

On Wed, Mar 05, 2025 at 06:14:16PM +1030, Qu Wenruo wrote:
> [...]
> >>>        /* spare for later */
> >>>        __u32 unused[4];
> >>
> >> We have enough space left here, although u32 is overkilled for
> >> compress_type, using the unused space for a new s8/s16/s32 member should
> >> be fine.
> > 
> > That is what I did originally, but discussing with Dave he suggested
> > this solution.
> 
> Normally I would be fine with the union, to save some memory.
> 
> Maybe I'm a little paranoid, but the defrag ioctl flag check is only 
> introduced last year by commit 173431b274a9 ("btrfs: defrag: reject 
> unknown flags of btrfs_ioctl_defrag_range_args").

The commit has been backported to stable trees 4.19.307 5.10.210 5.15.149
5.4.269 6.1.76 6.6.15 6.7.3 , so we could assume the flags are
validated.

> So it's possible that some older kernels don't have that commit, and may 
> incorrectly continue by ignoring the flag.
> Thankfully that should fail with -EINVAL (type always in the higher 
> bits, thus always tricking the NR_COMPRESS_TYPES check.
> 
> If that layout (type in higher bits, level in lower bits) is 
> intentionally, I'd say it's very clever.
> 
> Anyway either solution looks fine to me now.

The layout also depends on endianness, but should not matter as long as
the flgags are validated. If not, either the level is ignored or it
fails due to the >= NR_COMPRESS_TYPES check. Both should be acceptable
as fallback.