Re: [PATCH 03/13] blk-crypto: reject fallback data unit sizes it can't handle

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] block: NULL pointer deref in blk_crypto_fallback_evict_key() on setup failure
--

commit 9e7475fafabf0ae30868c1ceef29977b6846a713
Author: Baokun Li <libaokun@linux.alibaba.com>

    blk-crypto: reject fallback data unit sizes it can't handle

This patch restricts the data unit sizes advertised by blk-crypto-fallback
to only those it actually supports. It also rejects unsupported data unit
sizes early when setting up the key, preventing failures during I/O.

> diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c
> index 2a8f40a65158b..a75daadba4c86 100644
> --- a/block/blk-crypto-fallback.c
> +++ b/block/blk-crypto-fallback.c

[ ... ]

> @@ -612,16 +616,24 @@ static int blk_crypto_fallback_init(void)
>  }
>  
>  /*
> - * Prepare blk-crypto-fallback for the specified crypto mode.
> - * Returns -ENOPKG if the needed crypto API support is missing.
> + * Prepare blk-crypto-fallback for the specified crypto configuration.
> + * Returns -EOPNOTSUPP if the fallback can't handle the data unit size of the
> + * configuration, or -ENOPKG if the needed crypto API support is missing.
>   */
> -int blk_crypto_fallback_start_using_mode(enum blk_crypto_mode_num mode_num)
> +int blk_crypto_fallback_start_using_key(const struct blk_crypto_config *cfg)
>  {
> +	enum blk_crypto_mode_num mode_num = cfg->crypto_mode;
>  	const char *cipher_str = blk_crypto_modes[mode_num].cipher_str;
>  	struct blk_crypto_fallback_keyslot *slotp;
>  	unsigned int i;
>  	int err = 0;
>  
> +	if (!(BLK_CRYPTO_FALLBACK_DU_MASK & cfg->data_unit_size)) {
> +		pr_warn_ratelimited("can't handle data unit size %u\n",
> +				    cfg->data_unit_size);
> +		return -EOPNOTSUPP;
> +	}
> +

[Severity: High]
Does this early return bypass blk_crypto_fallback_init() and leave
blk_crypto_fallback_profile as NULL?

If this is the first fallback usage, returning here means we haven't
initialized the profile. If downstream callers (like dm-inlinecrypt) handle
this -EOPNOTSUPP error by attempting to evict the partially initialized key,
can it lead to a NULL pointer dereference?

When target teardown calls blk_crypto_evict_key(), it will eventually call
blk_crypto_fallback_evict_key(), which passes the uninitialized profile
directly:

block/blk-crypto-fallback.c:blk_crypto_fallback_evict_key() {
	return __blk_crypto_evict_key(blk_crypto_fallback_profile, key);
}

This unvalidated profile pointer is then dereferenced when checking the number
of slots:

block/blk-crypto-profile.c:__blk_crypto_evict_key() {
	...
	if (profile->num_slots == 0) {
	...
}

Could this cause a kernel panic if a user with device mapper access requests
an unsupported data unit size?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011170958.1759230-1-libaokun@linux.alibaba.com?part=3