linux-kernel - Re: [PATCH v4 10/24] crypto: x86/poly

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 for Android: free password hash cracker in your pocket

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

Date:   Tue, 06 Dec 2022 14:03:31 +0000
From:   Peter Lafreniere <peter@...jl.ca>
To:     Herbert Xu <herbert@...dor.apana.org.au>
Cc:     "Elliott, Robert (Servers)" <elliott@....com>,
        "Jason A. Donenfeld" <Jason@...c4.com>,
        "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>,
        "David.Laight@...lab.com" <David.Laight@...lab.com>,
        "ebiggers@...nel.org" <ebiggers@...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 v4 10/24] crypto: x86/poly - limit FPU preemption

> > > BTW, just a minor nit but you can delete the cond_resched() call
> > > because kernel_fpu_end()/preempt_enable() will do it anyway.
> > 
> > That happens under
> > CONFIG_PREEMPTION=y
> > (from include/Linux/preempt.h and arch/x86/include/asm/preempt.h)
> > 
> > Is calling cond_resched() still helpful if that is not the configuration?
> 
> 
> Perhaps, but then again perhaps if preemption is off, maybe we
> shouldn't even bother with the 4K split. Were the initial
> warnings with or without preemption?
> 
> Personally I don't really care since I always use preemption.
> 
> The PREEMPT Kconfigs do provide a bit of nuance with the split
> between PREEMPT_NONE vs. PREEMPT_VOLUNTARY. But perhaps that is
> just overkill for our situation.

I was thinking about this a few days ago, and my 2¢ is that it's 
probably best to not preempt the kernel in the middle of a crypto 
operation under PREEMPT_VOLUNTARY. We're already not preempting during 
these operations, and there haven't been complaints of excessive latency 
because of these crypto operations.

If we skip the kernel_fpu_{begin,end} pair when not under 
CONFIG_PREEMPT, we'll save a significant cycle count that is wasted 
currently. See Elliot Robert's numbers on conditional begin/end in sha 
to see the benefits of not saving/restoring unnecessarily: "10% of the 
CPU cycles spent making the [kernel_fpu_{begin,end}] calls".

> I'll leave it to you to decide :)

One extra thought: commit 827ee47: "crypto: x86 - add some helper macros 
for ECB and CBC modes" makes a mention of fpu save/restore being done 
lazily. I don't know the details, so would that change this discussion?

Thanks for listening,

Peter Lafreniere <peter@...jl.ca>