Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 21 Sep 2022 10:38:50 +0100
From:   Giovanni Cabiddu <giovanni.cabiddu@intel.com>
To:     <herbert@gondor.apana.org.au>, Adam Guerin <adam.guerin@intel.com>
CC:     <linux-crypto@vger.kernel.org>, <qat-linux@intel.com>,
        Ciunas Bennett <ciunas.bennett@intel.com>
Subject: Re: [PATCH v2] crypto: qat - add limit to linked list parsing
Message-ID: <Yyrbqno1KF8T+oCg@gcabiddu-mobl1.ger.corp.intel.com>
References: <20220921090923.213968-1-adam.guerin@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20220921090923.213968-1-adam.guerin@intel.com>
Organization: Intel Research and Development Ireland Ltd - Co. Reg. #308263 -
 Collinstown Industrial Park, Leixlip, County Kildare - Ireland
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?C4G8Fn6fURoN8uY+LbzzsJA0KbadX7LLqvechlPuTy7Af56RXvgN8KODdTl1?=
 =?us-ascii?Q?5Pg1owtw0u7QxQS6/ja286JpmNLIPOXi5NmSbrDJ0ZvWo+ukKxvfLzd68YjV?=
 =?us-ascii?Q?YfdS5/5aywbQNCJIpPLz6uCWiag9iVGH0jesRBxRJqkTdF+tR/QSOPoS/l5J?=
 =?us-ascii?Q?xaX9gXYymUDSrt597DsbYvifDKCyvfpAEbz4EnEwENzh+LegAWnW5rqJBSta?=
 =?us-ascii?Q?uq5Gkx0Yn7DPczWdhPkJCTw9Ied8aJ88p9SxFIqdi+bv3iW/qgOF0yTLYjh0?=
 =?us-ascii?Q?5zkviAGsM120/nsknV3HmFVmREiHhm3eLNmbp9E3gYunS2G78kjhrRlwFddc?=
 =?us-ascii?Q?2K5MdM1SqHUiVuo6Bz4zncxCAe1EBRWyp8xxZhR0x/okUT6ks1YLPwS6I7Om?=
 =?us-ascii?Q?3/QRWsMEvirO/U7QDv5/gI7j60p8hCBH8zm1kpd4gYA1Ke1D7JI7Ur2iypQB?=
 =?us-ascii?Q?W0G7y3YQbQgyyAstJtrY1SJW9jWudHt2BIX7RjUf7KhHxkBc9vnMG6fcqJN6?=
 =?us-ascii?Q?x5sJdX8572sb75iLF+d/Jagwi4pLT8/G9hJmhH22l3OP7vX1Bs8ZJn4xMvxT?=
 =?us-ascii?Q?PzxrBV/5eH8zZPivnNh7Uc4M28yq4BgQ0cSe1WTUfqe0T3ErLV5JixfTr03J?=
 =?us-ascii?Q?51BMlLJLOL2btJAoSJl6iMGqi7aTb1Lo6oHELw6iS89cpKysrWsTxVvCvUFb?=
 =?us-ascii?Q?DZmioaf/KVVjas195kwPDzhTw8qt5JBAaQuIhYLKJC7j3uWDz6vhVDYp67Ad?=
 =?us-ascii?Q?1jBq3Mo4Ey4/qroDDGjl7R9CiLXYx6WEZ28W7Q+dba9TohPiq/46Bzd+OpkB?=
 =?us-ascii?Q?oLm0eYb5+lU4ag6zGNnr9cJWwNFxNb8ISjdhBtejeAA89gzOhVTiq9PbfZkh?=
 =?us-ascii?Q?cy9XYlGpAp4ytsoz6OP/pe0Ga1JJdZI3pYQh4hV9uMUcY3c3Ei2T1RsqLiOw?=
 =?us-ascii?Q?roWOY25m4kIotRFsAqlCywFejFon46q778FvT9Jt/SykmYmY0EL6dIvqII/1?=
 =?us-ascii?Q?Wmp6FZ0Y2dpIjYz95N8ht+1TgVjoOwA5t9nfh1HPsR4aDFisboG7c3PCs05R?=
 =?us-ascii?Q?FDzR6R96OaohejFV7Jk/zpnZ0BXQ3icH9WSzwi9L3pDG4nYIjq/KSKM6irTn?=
 =?us-ascii?Q?w2wDJK1oU656nnkCFyx7U8Z2dBXEYq67kbiTbITrg5gH5dyYyB7qw1PmVO0l?=
 =?us-ascii?Q?klM1n/BuYpg/ugdOBz/OUzxcK0P5ymzbZ5YCZklOm6VswGad9wsWtQp/3O78?=
 =?us-ascii?Q?BDj13+6gl647G1oxxWCRzkqJa9bhRytlcM5HpmNgAUZUlhdkwr/AIJf2j2Jz?=
 =?us-ascii?Q?WBniooqJCjQujt5u44FWo38Lwc4WvKgxm8LINcltnKexrCGSTqZqO4/OMUMg?=
 =?us-ascii?Q?Bo5f1AL8mj+ZGAlcwGTVW+/41gAFWB6JgDy/ciq4XzsQdiPDSWVfguNGOvFt?=
 =?us-ascii?Q?av9uAI/ezEWmN8UpR93FyiUSXBksGiw9XxiTp8i5SrKW7RsJdzFKq1IFFprV?=
 =?us-ascii?Q?sozH7FR/fPrZrE2a1ofjVOonRgMM5zsfDE/mafBu4l2I65coc5RKvgN1Th5E?=
 =?us-ascii?Q?bEPmqcZwWCV7JtmrgEgZWqzIXziSnlwwKDMm6I75ACBOIQYZoXE3XdU+yS10?=
 =?us-ascii?Q?+A=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 8c314200-bb29-48f7-048e-08da9bb51ab4
X-MS-Exchange-CrossTenant-AuthSource: CY5PR11MB6366.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Sep 2022 09:38:56.8718
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: +R/XQoG0LO4rMgs6bN99PasMDgk8yVZyKqCzk1HxtoVsGBj05gRMXVZSNc6CZgWJCpUMdKyFhgRN1Qv5iAAh0GT1i8i6RFPJ6RaQyTZlg04=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB6920
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org

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@intel.com>
> Co-developed-by: Ciunas Bennett <ciunas.bennett@intel.com>
> Signed-off-by: Ciunas Bennett <ciunas.bennett@intel.com>
> Reviewed-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
> ---
> 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
>