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: <10b0cfa5-eadb-48a7-806a-3fc044682b04@gmx.com>
Date: Wed, 5 Mar 2025 18:38:49 +1030
From: Qu Wenruo <quwenruo.btrfs@....com>
To: dsterba@...e.cz, Qu Wenruo <wqu@...e.com>
Cc: Daniel Vacek <neelx@...e.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



在 2025/3/5 18:31, David Sterba 写道:
> 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.

I know it's backported to all stable kernels, but I still won't consider
a patch that's only one year old to be applied to all the kernels in the
wide.

Maybe I'm really paranoid on this.

>
>> 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.

Since the fallback behavior is sane, I'm fine with the union solution.

Thanks,
Qu