Re: [PATCH 1/4] fscrypt: Cap data units at the page size

[Baokun Li]

Hi Eric,

Thank you for your feedback.

On 2026/11/4 06:40, Eric Biggers wrote:
> On Tue, Nov 03, 2026 at 11:31:29PM +0800, Baokun Li wrote:
>> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
>> index e602c31b3844..0dc158563e71 100644
>> --- a/fs/crypto/fscrypt_private.h
>> +++ b/fs/crypto/fscrypt_private.h
>> @@ -203,7 +203,9 @@ static inline int
>>  fscrypt_policy_v2_du_bits(const struct fscrypt_policy_v2 *policy,
>>  			  const struct inode *inode)
>>  {
>> -	return policy->log2_data_unit_size ?: inode->i_blkbits;
>> +	if (policy->log2_data_unit_size)
>> +		return policy->log2_data_unit_size;
>> +	return min_t(unsigned int, inode->i_blkbits, PAGE_SHIFT);
>>  }
> 
> This would make the interpretation of the default value
> log2_data_unit_size=0 depend on the page size, which (unlike i_blkbits)
> is not part of the filesystem's on-disk format.


This is indeed a point we struggled with before.

The problem is that on a filesystem with LBS enabled, once fscrypt is
enabled, log2_data_unit_size=0 causes data corruption.  So we default
to a sub-block data unit size to avoid it.

In the previous discussion it was said that for cross-machine
compatibility, users would explicitly set a data unit size of 4K or
smaller.  So this change did not consider cross-machine or cross-page-
size compatibility.

> 
> I think that for a sub-block data unit size to be used, it should need
> to continue to be explicitly specified.  Otherwise the default would
> become ambiguous.

This does change the semantics of the default value, and supporting a
data unit size larger than the page size later would be even worse.

What I had in mind before was to keep the default as min(bs, PS) and
allow DU > PS only when explicitly specified, which seemed to give
better compatibility.  Cross-machine or cross-page-size cases seem
uncommon, and existing machines have probably already avoided them by
specifying a 4K or smaller DU.

Keeping the existing default semantics and returning an error in the
LBS case for the user to specify seems feasible.  But then
test_dummy_encryption and the xfstests cases that use the default would
need to specify the DU explicitly, otherwise the LBS + encrypt test
coverage would drop a lot.  That is, in the kernel or the tests we
always have to set a default DU size.

If you confirm that we should take the route where LBS requires an
explicit data unit size and the default is rejected, I can adapt the
kernel and xfstests in v2.  Or should we go back to the more compatible
DU > page size approach?

Looking forward to hearing your thoughts.


Cheers,
Baokun