linux-kernel - Re: [PATCH] ext4: fix to report fstrim.minlen back to userspace

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] [day] [month] [year] [list]

Message-ID: <265f6c37-fae7-6ab2-594f-e7785aedb4e6@kernel.org>
Date:   Sun, 12 Mar 2023 18:15:42 +0800
From:   Chao Yu <chao@...nel.org>
To:     Theodore Ts'o <tytso@....edu>
Cc:     adilger.kernel@...ger.ca, linux-ext4@...r.kernel.org,
        linux-kernel@...r.kernel.org, Lukas Czerner <lczerner@...hat.com>
Subject: Re: [PATCH] ext4: fix to report fstrim.minlen back to userspace

On 2023/3/11 11:18, Theodore Ts'o wrote:
> Unfortunately, this patch is not correct.  The units of struct
> fstrim_range's minlen (here, range->minlen) is bytes.

Oh, that's right, sorry for the mistake.

> 
> However the minlen variable in ext4_trim_fs is in units of *clusters*.
> And so it gets rounded up two places.  The first time is when it is
> converted into units of a cluster:
> 
> 	minlen = EXT4_NUM_B2C(EXT4_SB(sb),
> 			      range->minlen >> sb->s_blocksize_bits);
IIUC, if range->minlen is smaller than block size of ext4, above calculation
may return a wrong value, due to it looks EXT4_NUM_B2C() expects a non-zero
in-parameter.

So it needs to round up minlen to block size first and then round up block
size to cluster size:

	minlen =  EXT4_NUM_B2C(EXT4_SB(sb),
		EXT4_BLOCK_ALIGN(range->minlen, sb->s_blocksize_bits));

Or do the conversion at a time as you reminded:

	minlen = (range->minlen + EXT4_CLUSTER_SIZE(sb) - 1) >>
		(sb->s_blocksize_bits + EXT4_CLUSTER_BITS(sb));

> 
> And the second time is when it is rounded up to the block device's
> discard granularity.
> 
> So after that if statement, we need to convert minlen from clusters to
> bytes, like so:
> 
> 	range->minlen = EXT4_C2B(EXT4_SB(sb), minlen) << sb->s_blocksize_bits);

Thanks for the detailed explanation and reminder. :)

Thanks,