linux-kernel - RE: [PATCH v2 05/19] crypto: x86/crc

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: <MW5PR84MB1842FCF31D9CE954873250BCAB259@MW5PR84MB1842.NAMPRD84.PROD.OUTLOOK.COM>
Date:   Thu, 13 Oct 2022 22:34:02 +0000
From:   "Elliott, Robert (Servers)" <elliott@....com>
To:     Herbert Xu <herbert@...dor.apana.org.au>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "tim.c.chen@...ux.intel.com" <tim.c.chen@...ux.intel.com>,
        "ap420073@...il.com" <ap420073@...il.com>,
        "ardb@...nel.org" <ardb@...nel.org>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 05/19] crypto: x86/crc - limit FPU preemption



> -----Original Message-----
> From: Herbert Xu <herbert@...dor.apana.org.au>
> Sent: Wednesday, October 12, 2022 9:00 PM
> To: Elliott, Robert (Servers) <elliott@....com>
> Cc: davem@...emloft.net; tim.c.chen@...ux.intel.com; ap420073@...il.com;
> ardb@...nel.org; linux-crypto@...r.kernel.org; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH v2 05/19] crypto: x86/crc - limit FPU preemption
> 
> On Wed, Oct 12, 2022 at 04:59:17PM -0500, Robert Elliott wrote:
> >
> > @@ -170,9 +179,17 @@ static int __crc32c_pcl_intel_finup(u32 *crcp, const u8
> *data, unsigned int len,
> >  				u8 *out)
> >  {
> >  	if (len >= CRC32C_PCL_BREAKEVEN && crypto_simd_usable()) {
> > -		kernel_fpu_begin();
> > -		*(__le32 *)out = ~cpu_to_le32(crc_pcl(data, len, *crcp));
> > -		kernel_fpu_end();
> > +		do {
> > +			unsigned int chunk = min(len, FPU_BYTES);
> > +
> > +			kernel_fpu_begin();
> > +			*crcp = crc_pcl(data, chunk, *crcp);
> 
> How about storing the intermediate result in a local variable
> instead of overwriting *crcp?
> 
> Thanks,

The _update function does so, and it's not marked const here,
so seemed prudent to keep up to date.

Do the callers understand it's no longer valid after finup, or
is there any case they might treat finup like an update and try
again?