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

[Baokun Li <libaokun@linux.alibaba.com>]

On 2026/11/5 03:51, Eric Biggers wrote:
> On Wed, Nov 04, 2026 at 10:31:46AM +0800, Baokun Li wrote:
>> 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?
> 
> It will indeed be annoying to have to explicitly specify the data unit
> size, but I don't see a better solution for now, given the complexity of
> supporting data unit sizes larger than the page size.  I think making
> the on-disk format needing PAGE_SIZE to interpret is a no-go.
> 

Agreed.

> I suppose that in theory, how FS_IOC_SET_ENCRYPTION_POLICY handles
> log2_data_unit_size=0 could be changed: it could start actually writing
> log2_data_unit_size=12 to disk (rather than writing 0 which means the
> filesystem block size).  I think that can still lead to problems,
> though.  For one FS_IOC_GET_ENCRYPTION_POLICY would return something
> different from what was set.

I also considered making the default explicit, e.g. 4096, when the
bs > PS.  GET_ENCRYPTION_POLICY would then report 4096 instead of
the 0 that was set, which I think is fine provided we log it:
an unset field means "use the kernel default", and this just pins
down what that default is.  This works for v2.

I dropped it over test_dummy_encryption: mount options are parsed
before the block size is known, and the policy is const once it is,
so only invasive changes would work.

But the dummy policy is recreated on every mount and never persisted,
so there is no on-disk policy to stay compatible with.  The template
can just use SECTOR_SHIFT (simplest); alternatively, refuse it on such
a fs, or support an explicit du_size through a helper shared by the fs
(most complex).

> 
> Regarding xfstests, take a look at how common/verity adds $FSV_BLOCKSIZE
> to all 'fsverity enable' commands by default, to select some Merkle tree
> block size that the filesystem supports.  Something similar could be
> done with fscrypt.  Yes, this won't work with v1 encryption policies,
> but v1 policies are obsolete.  Most of the xfstests that use v1 policies
> should just be updated to use v2 instead.  The btrfs folks were working
> on that earlier, since they're not planning to support v1 at all.
> 

Agreed on rejecting v1.  The xfstests cases that need it will have to
be skipped or converted to v2; I can do that if nobody else is on it.

So, to enable this by default when bs > PS:

  1. reject v1, and convert the xfstests cases;
  2. in fscrypt_ioctl_set_policy(), make the default 4096 explicit
     and log it;
  3. for dummy, either use SECTOR_SHIFT, refuse it on such a
     fs and skip the tests, or support an explicit du_size?

Looking forward to hearing your thoughts.


Cheers,
Baokun