Re: [PATCH v2 2/4] btrfs: also validate compression levels in btrfs_compress_is_valid_type()

["koraynilay" <koray.fra@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1382 bytes --]

On Sun Aug 9, 2026 at 4:53 AM CEST, Qu Wenruo wrote:
> Please make sure every commit compiles, this will easily break bisection.
>
> You're changing a function prototype and implementation without 
> modifying any callers, this fails compiling.

Right, I'm sorry, I didn't notice I changed the caller in 3/4 as that
call point also had other changes.

> Furthermore, I do not think it's a good idea to just rely on the strlen().
>
> E.g. if a crafted image removing the last terminating NUL, relying 
> strlen() can easily go beyond the expected string.
>
> I think the change to remove @len is going to reduce the robustness of 
> the original code.

That was what I feared too, the problem is that
btrfs_compress_str2level() uses kstrtoint(), which requires the string
to be null-terminated[1], so the len paramenter would only be used by
if (len < comp_len) and that reduces the robustness even more IMO, since
a developer could think it's safe because of len, while it actually
isn't because of kstrtoint().
The only fix I can think of would be to use
_parse_integer_limit(..., len) directly or write a wrapper.
Or just document that it needs a null-terminated string.

Thanks.

Best,
koraynilay

[1]: 
from lib/kstrtox.c:251:
 * kstrtoint - convert a string to an int
 * @s: The start of the string. The string must be null-terminated [...]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]