Re: [PATCH v2] KEYS: Protect pkey method checks against type removal

[Jarkko Sakkinen <jarkko@kernel.org>]

Please respond to any questions *before* sending a follow-up patch by
properly using email client and quoting etc.

https://docs.kernel.org/process/email-clients.html

On Wed, Nov 04, 2026 at 10:14:14AM +0800, BeaCox wrote:
> @@ -129,7 +126,14 @@ static int keyctl_pkey_params_get_2(const struct keyctl_pkey_params __user *_par
>  	if (ret < 0)
>  		return ret;
>  
> +	down_read(&params->key->sem);
> +	ret = -EOPNOTSUPP;
> +	if (!params->key->type->asym_query) {
		ret -EOPNOTSUPP;
> +		goto out_unlock;
	}
	

> +
>  	ret = params->key->type->asym_query(params, &info);

There should be an empty line prepending the label.

> +out_unlock:
> +	up_read(&params->key->sem);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -185,15 +189,25 @@ long keyctl_pkey_query(key_serial_t id,
>  	if (ret < 0)
>  		goto error;
>  
> +	down_read(&params.key->sem);
> +	ret = -EOPNOTSUPP;
> +	if (!params.key->type->asym_query)
> +		goto error_unlock;

Ditto.

> +
>  	ret = params.key->type->asym_query(&params, &res);
>  	if (ret < 0)
> -		goto error;
> +		goto error_unlock;
> +
> +	up_read(&params.key->sem);
>  
>  	ret = -EFAULT;
>  	if (copy_to_user(_res, &res, sizeof(res)) == 0 &&
>  	    clear_user(_res->__spare, sizeof(_res->__spare)) == 0)
>  		ret = 0;

Why up_read is not here?

> +	goto error;  
> +error_unlock:
> +	up_read(&params.key->sem);
>  error:
>  	keyctl_pkey_params_free(&params);
>  	return ret;
> @@ -230,9 +244,21 @@ long keyctl_pkey_e_d_s(int op,
>  	if (ret < 0)
>  		goto error_params;
>  
> +	in = memdup_user(_in, params.in_len);
> +	if (IS_ERR(in)) {
> +		ret = PTR_ERR(in);
> +		goto error_params;
> +	}
> +
> +	ret = -ENOMEM;
> +	out = kmalloc(params.out_len, GFP_KERNEL);
> +	if (!out)
> +		goto error_in;
> +
> +	down_read(&params.key->sem);
>  	ret = -EOPNOTSUPP;
>  	if (!params.key->type->asym_eds_op)
> -		goto error_params;
> +		goto error_unlock;
>  
>  	switch (op) {
>  	case KEYCTL_PKEY_ENCRYPT:
> @@ -246,27 +272,18 @@ long keyctl_pkey_e_d_s(int op,
>  		break;
>  	default:
>  		ret = -EOPNOTSUPP;
> -		goto error_params;
> +		goto error_unlock;
>  	}
>  
> -	in = memdup_user(_in, params.in_len);
> -	if (IS_ERR(in)) {
> -		ret = PTR_ERR(in);
> -		goto error_params;
> -	}
> -
> -	ret = -ENOMEM;
> -	out = kmalloc(params.out_len, GFP_KERNEL);
> -	if (!out)
> -		goto error_in;
> -
>  	ret = params.key->type->asym_eds_op(&params, in, out);
> -	if (ret < 0)
> -		goto error_out;
> +	up_read(&params.key->sem);
>  
> -	if (copy_to_user(_out, out, ret) != 0)
> +	if (ret >= 0 && copy_to_user(_out, out, ret) != 0)
>  		ret = -EFAULT;
> +	goto error_out;

Please figure this part out it does not do a great job minimizing the diff.

There were similar repeating issues that I did not specifically point
out.

Br, Jarkko