linux-kernel - Re: [PATCH] crypto: qat - use strscpy_pad to simplify buffer initialization

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aPs9w2su33uXfD09@gcabiddu-mobl.ger.corp.intel.com>
Date: Fri, 24 Oct 2025 09:50:11 +0100
From: Giovanni Cabiddu <giovanni.cabiddu@...el.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
CC: Thorsten Blum <thorsten.blum@...ux.dev>, Herbert Xu
	<herbert@...dor.apana.org.au>, "David S. Miller" <davem@...emloft.net>, "Jack
 Xu" <jack.xu@...el.com>, Suman Kumar Chakraborty
	<suman.kumar.chakraborty@...el.com>, Qianfeng Rong <rongqianfeng@...o.com>,
	<qat-linux@...el.com>, <linux-crypto@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] crypto: qat - use strscpy_pad to simplify buffer
 initialization

On Thu, Oct 23, 2025 at 09:44:01PM +0300, Andy Shevchenko wrote:
> On Thu, Oct 23, 2025 at 05:35:00PM +0200, Thorsten Blum wrote:
> > On 22. Oct 2025, at 20:17, Andy Shevchenko wrote:
> > > On Wed, Oct 22, 2025 at 02:36:19PM +0200, Thorsten Blum wrote:
> 
> ...
> 
> > How about this?
> 
> LGTM, and that's what I had in mind, but please double check the behaviour of
> kstrtox() on an empty strings.

LGTM as well.

I checked the behaviour of kstrtoul() when given an empty string. It
returns -EINVAL (-22). The result variable (the third parameter) is only
modified if the conversion is succesful.

Anyway, the caller will not provide a NULL string to this function [1].

> > diff --git a/drivers/crypto/intel/qat/qat_common/qat_uclo.c b/drivers/crypto/intel/qat/qat_common/qat_uclo.c
> > index 18c3e4416dc5..04628dc01456 100644
> > --- a/drivers/crypto/intel/qat/qat_common/qat_uclo.c
> > +++ b/drivers/crypto/intel/qat/qat_common/qat_uclo.c
> > @@ -200,20 +200,12 @@ qat_uclo_cleanup_batch_init_list(struct icp_qat_fw_loader_handle *handle,
> >  
> >  static int qat_uclo_parse_num(char *str, unsigned int *num)
> >  {
> > -	char buf[16] = {0};
> > -	unsigned long ae = 0;
> > -	int i;
> > -
> > -	strscpy(buf, str, sizeof(buf));
> > -	for (i = 0; i < 16; i++) {
> > -		if (!isdigit(buf[i])) {
> > -			buf[i] = '\0';
> > -			break;
> > -		}
> > -	}
> > -	if ((kstrtoul(buf, 10, &ae)))
> > -		return -EFAULT;
> > +	unsigned long long ae;
> > +	char *end;
> >  
> > +	ae = simple_strtoull(str, &end, 10);
> > +	if (ae > UINT_MAX || str == end || (end - str) > 20)
> 
> I would go with >= 20, the 64-bit value is approx. 1 * 10^19.

Just an insight into the type of strings being parsed here. If they are
well-formed, the format looks like:

    <AE_NUMBER>!<...>

Example:

    11!l0000!lm_thread_ctrl_struct_base

This function just extract the first number. Currently, that is 2 digits [2],
and I believe it is unlikely to exceed 3 in future gens.

[1] https://elixir.bootlin.com/linux/v6.17.4/source/drivers/crypto/intel/qat/qat_common/qat_uclo.c#L237
[2] https://elixir.bootlin.com/linux/v6.17.4/source/drivers/crypto/intel/qat/qat_common/icp_qat_uclo.h#L11

Regards,

-- 
Giovanni