Re: [PATCH v2 2/2] blk-crypto: fix missing smp_load_acquire() for tfms_inited[]

[Eric Biggers <ebiggers@kernel.org>]

On Fri, Oct 16, 2026 at 01:36:17AM +0800, Baokun Li wrote:
> That's what I was about to do for v3, but while looking for other
> callers that inherit a key without calling start_using_key()
> themselves, I found that dm-inlinecrypt has the same shape, so fixing
> fscrypt alone wouldn't cover it -- only the constructor calls
> start_using_key(), and the task submitting I/O reaches the key through
> md->map:
> 
>   dmsetup (ctr)                      I/O task
>   ---------------------------------- -------------------------------
>   blk_crypto_start_using_key()
>    fallback_start_using_mode()
>     init profile, keyslots, tfms[]
>     smp_store_release(tfms_inited[m])
>   rcu_assign_pointer(md->map, t)
>                                      srcu_dereference(md->map)
>                                       -> ti->private -> &ctx->key
>                                      inlinecrypt_map() -> bio_prep()
>                                        tfms_inited[m]  addr-dep, ok
>                                        profile/keyslots/tfms[]  not
> 
> dm's own grace periods would cover this, but all three are conditional:
> 
>   drivers/md/dm.c:2533       if (old_map) dm_sync_table(md);
>   drivers/md/dm.c:3015,3039  if (map) synchronize_srcu(&md->io_barrier);
>   drivers/md/dm-ioctl.c:1651 add_disk() only when type == DM_TYPE_NONE
> 
> so e.g. this sequence has no happens-before:
> 
>   DM_DEV_CREATE
>   DM_TABLE_LOAD (linear)       add_disk(), md->type set, not resumed
>                                  -> a task opens the device here
>   DM_TABLE_CLEAR
>   DM_TABLE_LOAD (inlinecrypt)  ctr sets tfms_inited[]; no add_disk()
>   DM_DEV_SUSPEND (resume)      old_map == NULL, all three skipped
>                                  -> the already-open task does I/O
> 
> This needs a weakly ordered machine and a deliberate ioctl order, so
> it's not urgent.  Which would you prefer?
> 
>   1. Leave it for now and fix only fscrypt_find_mode_key().
> 
>   2. A separate smp_rmb() in inlinecrypt_map(), paired with the
>      rcu_assign_pointer(md->map) in __bind().  Keeps the cost on
>      dm-inlinecrypt only, but means a second site to get right.
> 
>   3. Keep the barrier in blk_crypto_fallback_bio_prep() after all,
>      documented as ordering the loads after the check rather than the
>      flag itself.  One site covers both callers and any future one,
>      at the cost of blk-crypto no longer relying on the caller for
>      this part.

So the device-mapper core doesn't order target_type::ctr before
target_type::map?  Nearly every dm target's map function relies on the
constructor having run first.  If this is an issue, it isn't specific to
dm-inlinecrypt and it will need to be fixed in the device-mapper core.

- Eric