Re: [PATCH v3 0/7] Fix bio splitting by the crypto fallback code

[Eric Biggers <ebiggers@kernel.org>]

On Fri, Jul 18, 2025 at 08:37:04AM -0700, Bart Van Assche wrote:
> On 7/16/25 9:43 PM, Christoph Hellwig wrote:
> > Getting back to this.  While the ton is a bit snarky, it brings up a good
> > point.  Relying on the block layer to ensure that data is always
> > encrypted seems like a bad idea, given that is really not what the block
> > layer is about, and definitively not high on the mind of anyone touching
> > the code.  So I would not want to rely on the block layer developers to
> > ensure that data is encrypted properly through APIs not one believes part
> > that mission.
> > 
> > So I think you'd indeed be much better off not handling the (non-inline)
> > incryption in the block layer.
> > 
> > Doing that in fact sounds pretty easy - instead of calling the
> > blk-crypto-fallback code from inside the block layer, call it from the
> > callers instead of submit_bio when inline encryption is not actually
> > supported, e.g.
> > 
> > 	if (!blk_crypto_config_supported(bdev, &crypto_cfg))
> > 		blk_crypto_fallback_submit_bio(bio);
> > 	else
> > 		submit_bio(bio);
> > 
> > combined with checks in the low-level block code that we never get a
> > crypto context into the low-level submission into ->submit_bio or
> > ->queue_rq when not supported.
> > 
> > That approach not only is much easier to verify for correct encryption
> > operation, but also makes things like bio splitting and the required
> > memory allocation for it less fragile.
> 
> Has it ever been considered to merge the inline encryption code into
> dm-crypt or convert the inline encryption fallback code into a new dm
> driver? If user space code would insert a dm device between filesystems
> and block devices if hardware encryption is not supported that would
> have the following advantages:
> * No filesystem implementations would have to be modified.
> * It would make it easier to deal with bio splitting since dm drivers
>   can set stacking limits in their .io_hints() callback.

Yes, this was considered back when blk-crypto-fallback was being added.
But it is useful to support blk-crypto unconditionally without userspace
having to set up a dm device:

- It allows testing the inline encryption code paths in ext4 and f2fs
  with xfstests on any system, just by adding the inlinecrypt mount
  option.  See e.g.
  https://lore.kernel.org/linux-block/20191031205045.GG16197@mit.edu/
  and 
  https://lore.kernel.org/linux-block/20200515170059.GA1009@sol.localdomain/

- It allows upper layers to just use blk-crypto instead of having both
  blk-crypto and non-blk-crypto code paths.  While I've been late on
  converting fscrypt to actually rely on this, it still might be a good
  idea, especially as we now need to revisit the code for reasons like
  large folio support.  (And Christoph seems to support this too -- see
  https://lore.kernel.org/linux-block/20250715062112.GC18672@lst.de/)

But, as suggested at
https://lore.kernel.org/linux-block/20250717044342.GA26995@lst.de/ it
should also be okay to reorganize things so that the regular
submit_bio() does not support the fallback, and upper layers have to
call a different function blk_crypto_fallback_submit_bio() if they want
the fallback.  I don't think that would help with the splitting issue
directly, but perhaps we could make the filesystems just not submit bios
that would need splitting by blk-crypto-fallback, which would solve the
issue.

- Eric