Re: [PATCH v2] crypto: qat - add limit to linked list parsing

[Giovanni Cabiddu <giovanni.cabiddu@xxxxxxxxx>]

Hi Herbert,

This patch was accidentally sent starting from V2. Adam is going to
resend.

Regards,

-- 
Giovanni

On Wed, Sep 21, 2022 at 10:09:24AM +0100, Adam Guerin wrote:
> adf_copy_key_value_data() copies data from userland to kernel, based on
> a linked link provided by userland. If userland provides a circular
> list (or just a very long one) then it would drive a long loop where
> allocation occurs in every loop. This could lead to low memory conditions.
> Adding a limit to stop endless loop.
> 
> Signed-off-by: Adam Guerin <adam.guerin@xxxxxxxxx>
> Co-developed-by: Ciunas Bennett <ciunas.bennett@xxxxxxxxx>
> Signed-off-by: Ciunas Bennett <ciunas.bennett@xxxxxxxxx>
> Reviewed-by: Giovanni Cabiddu <giovanni.cabiddu@xxxxxxxxx>
> ---
> v2: improved patch based off feedback from ML
> drivers/crypto/qat/qat_common/adf_ctl_drv.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/qat/qat_common/adf_ctl_drv.c b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
> index 508c18edd692..82b69e1f725b 100644
> --- a/drivers/crypto/qat/qat_common/adf_ctl_drv.c
> +++ b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
> @@ -16,6 +16,9 @@
>  #include "adf_cfg_common.h"
>  #include "adf_cfg_user.h"
>  
> +#define ADF_CFG_MAX_SECTION 512
> +#define ADF_CFG_MAX_KEY_VAL 256
> +
>  #define DEVICE_NAME "qat_adf_ctl"
>  
>  static DEFINE_MUTEX(adf_ctl_lock);
> @@ -137,10 +140,11 @@ static int adf_copy_key_value_data(struct adf_accel_dev *accel_dev,
>  	struct adf_user_cfg_key_val key_val;
>  	struct adf_user_cfg_key_val *params_head;
>  	struct adf_user_cfg_section section, *section_head;
> +	int i, j;
>  
>  	section_head = ctl_data->config_section;
>  
> -	while (section_head) {
> +	for (i = 0; section_head && i < ADF_CFG_MAX_SECTION; i++) {
>  		if (copy_from_user(&section, (void __user *)section_head,
>  				   sizeof(*section_head))) {
>  			dev_err(&GET_DEV(accel_dev),
> @@ -156,7 +160,7 @@ static int adf_copy_key_value_data(struct adf_accel_dev *accel_dev,
>  
>  		params_head = section.params;
>  
> -		while (params_head) {
> +		for (j = 0; params_head && j < ADF_CFG_MAX_KEY_VAL; j++) {
>  			if (copy_from_user(&key_val, (void __user *)params_head,
>  					   sizeof(key_val))) {
>  				dev_err(&GET_DEV(accel_dev),
> 
> base-commit: 8aee6d5494bfb2e535307eb3e80e38cc5cc1c7a6
> -- 
> 2.37.3
>