Re: [PATCH v5 6/9] crash_dump: Improve readability of config_keys_restore_store

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The local variable `r` is returned uninitialized when the user writes a value evaluating to false.
--

commit 22a99a886bc0e01618cd42c5f2b503de86fa5a10
Author: Coiby Xu <coiby.xu@xxxxxxxxx>

crash_dump: Improve readability of config_keys_restore_store

This commit introduces user input validation to config_keys_restore_store
before restoring dm-crypt keys. It ensures the restore function's return
code is verified and improves overall code readability.

> diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
> index 9ad50ac54866a..b8fe630462cfa 100644
> --- a/kernel/crash_dump_dm_crypt.c
> +++ b/kernel/crash_dump_dm_crypt.c
> @@ -371,13 +371,25 @@ static ssize_t config_keys_restore_show(struct config_item *item, char *page)
>  static ssize_t config_keys_restore_store(struct config_item *item,
>  					  const char *page, size_t count)
>  {
> -	if (!restore)
> -		restore_dm_crypt_keys_to_thread_keyring();
> +	bool val;
> +	int r;
>  
> -	if (kstrtobool(page, &restore))
> +	if (kstrtobool(page, &val))
>  		return -EINVAL;
>  
> -	return count;
> +	if (val) {
> +		if (restore) {
> +			pr_warn("dm-crypt keys already restored!\n");
> +			return count;
> +		}
> +		r = restore_dm_crypt_keys_to_thread_keyring();
> +		if (!r) {
> +			restore = true;
> +			r = count;
> +		}
> +	}
> +

[Severity: High]
If a user writes a value evaluating to false (e.g., '0' or 'n') to this
configfs attribute, kstrtobool() will succeed and set val to false.

This causes the if (val) block to be bypassed entirely, resulting in the
uninitialized variable r being returned here. Could this leak uninitialized
kernel stack memory to userspace or cause undefined behavior?

> +	return r;
>  }
>  
>  CONFIGFS_ATTR(config_keys_, restore);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909003657.1570544-1-coiby.xu@xxxxxxxxx?part=6