Re: [PATCH v3 08/10] crash_dump: Check the function return codes in restore_dm_crypt_keys_to_thread_keyring

On 29/07/26 09:06, Coiby Xu wrote:
> We should check the return codes so we can abort if keyring allocation
> or reading old memory fails.
>
> Note there is no need to refer a key add_key_to_keyring, so delete
> related code.
>
> Signed-off-by: Coiby Xu <[email protected]>
> ---
>   kernel/crash_dump_dm_crypt.c | 23 ++++++++++++++++-------
>   1 file changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
> index 1f4549956824..b5b78656cf06 100644
> --- a/kernel/crash_dump_dm_crypt.c
> +++ b/kernel/crash_dump_dm_crypt.c
> @@ -65,7 +65,7 @@ static int add_key_to_keyring(struct dm_crypt_key *dm_key,
>   			      key_ref_t keyring_ref)
>   {
>   	key_ref_t key_ref;
> -	int r;
> +	int r = 0;
>   
>   	/* create or update the requested key and add it to the target keyring */
>   	key_ref = key_create_or_update(keyring_ref, "user", dm_key->key_desc,
> @@ -73,8 +73,6 @@ static int add_key_to_keyring(struct dm_crypt_key *dm_key,
>   				       KEY_USR_ALL, KEY_ALLOC_IN_QUOTA);
>   
>   	if (!IS_ERR(key_ref)) {
> -		r = key_ref_to_ptr(key_ref)->serial;
> -		key_ref_put(key_ref);
>   		pr_debug("Success adding key %s", dm_key->key_desc);
>   	} else {
>   		r = PTR_ERR(key_ref);
> @@ -133,9 +131,14 @@ static int restore_dm_crypt_keys_to_thread_keyring(void)
>   	}
>   
>   	addr = dm_crypt_keys_addr;
> -	dm_crypt_keys_read((char *)&key_count, sizeof(key_count), &addr);
> +	ret = dm_crypt_keys_read((char *)&key_count, sizeof(key_count), &addr);
> +	if (ret < 0) {
> +		pr_err("Failed to read the number of dm-crypt keys\n");
> +		goto out;
> +	}
> +
>   	if (key_count > KEY_NUM_MAX) {
> -		pr_warn("Failed to read the number of dm-crypt keys\n");
> +		pr_warn("Read %u dm-crypt keys (max=%u)\n", key_count, KEY_NUM_MAX);
>   		ret = -1;
>   		goto out;
>   	}
> @@ -150,12 +153,18 @@ static int restore_dm_crypt_keys_to_thread_keyring(void)
>   		goto out;
>   	}
>   
> -	dm_crypt_keys_read((char *)keys_header, keys_header_size, &addr);
> +	ret = dm_crypt_keys_read((char *)keys_header, keys_header_size, &addr);
> +	if (ret < 0) {
> +		pr_err("Failed to read dm-crypt keys\n");
> +		goto out;
> +	}
>   
>   	for (int i = 0; i < keys_header->total_keys; i++) {
>   		key = &keys_header->keys[i];
>   		pr_debug("Get key (size=%u)\n", key->key_size);
> -		add_key_to_keyring(key, keyring_ref);
> +		ret = add_key_to_keyring(key, keyring_ref);
> +		if (ret)
> +			break;
>   	}
>   
>   out:

Changes looks good to me. Feel free to add.
Reviewed-by: Sourabh Jain <[email protected]>