lib/mpi: call cond_resched() from mpi_powm() loop
Subsystems (3)
PATCH v2 1/1 lib/mpi: call cond_resched() from mpi_powm() loop
From: Eric Biggers <[email protected]>
On a non-preemptible kernel, if KEYCTL_DH_COMPUTE is called with the largest permitted inputs (16384 bits), the kernel spends 10+ seconds doing modular exponentiation in mpi_powm() without rescheduling. If all threads do it, it locks up the system. Moreover, it can cause rcu_sched-stall warnings.
Notwithstanding the insanity of doing this calculation in kernel mode rather than in userspace, fix it by calling cond_resched() as each bit from the exponent is processed. It's still noninterruptible, but at least it's preemptible now.
Do the cond_resched() once per bit rather than once per MPI limb because each limb might still easily take 100+ milliseconds on slow CPUs.
--- lib/mpi/mpi-pow.c | 2 ++ 1 file changed, 2 insertions(+)
lib/mpi/mpi-pow.c +2 -0
@@ -26,6 +26,7 @@* however I decided to publish this code under the plain GPL. */ +#include <linux/sched.h> #include <linux/string.h> #include "mpi-internal.h" #include "longlong.h"
@@ -256,6 +257,7 @@ int mpi_powm(MPI res, MPI base, MPI exp, MPI mod) } e <<=1 ; c--; +cond_resched (); } i--;
Signature · 1 line
2.15.0.403.gc27cc4dac6-goog
David Howells
maintainerQuoted 5 lines · Eric Biggers
On a non-preemptible kernel, if
KEYCTL_DH_COMPUTEis called with the largest permitted inputs (16384 bits), the kernel spends 10+ seconds doing modular exponentiation inmpi_powm()without rescheduling. If all threads do it, it locks up the system. Moreover, it can cause rcu_sched-stall warnings.
Do you want this to go in immediately, or I should I push it to James for the next merge window?
David
Hi David,
Quoted 8 lines · David Howells
Quoted 5 lines · Eric Biggers
On a non-preemptible kernel, if
KEYCTL_DH_COMPUTEis called with the largest permitted inputs (16384 bits), the kernel spends 10+ seconds doing modular exponentiation inmpi_powm()without rescheduling. If all threads do it, it locks up the system. Moreover, it can cause rcu_sched-stall warnings.
Quoted 4 lines
Do you want this to go in immediately, or I should I push it to James for the next merge window?
David
This probably should be grouped with my series "crypto: dh - input validation fixes", as this is also a fix for Diffie-Hellman. I was actually expecting Herbert Xu to take these patches, as Diffie-Hellman is now part of the crypto API (crypto/dh.c); none of the patches actually touch security/keys/. If you'd like to maintain the Diffie-Hellman implementation(s) you should get yourself added to MAINTAINERS for the relevant files.
As for the criticality of the fixes, the code is new in v4.12, so I doubt many people have CONFIG_KEY_DH_OPERATIONS=y enabled yet. syzbot doesn't actually have it enabled either (we should turn it on); I just happened to find these bugs by running syzkaller myself. So it may not be particularly critical to take these fixes immediately. However, at least the fix for the double free bug ("crypto: dh - Fix double free of ctx->p") shouldn't wait for too long...
Eric
David Howells
maintainerQuoted 4 lines · Eric Biggers
This probably should be grouped with my series "crypto: dh - input validation fixes", as this is also a fix for Diffie-Hellman. I was actually expecting Herbert Xu to take these patches, as Diffie-Hellman is now part of the crypto API (
crypto/dh.c); none of the patches actually touchsecurity/keys/.
Fine by me.
Quoted 2 lines
If you'd like to maintain the Diffie-Hellman implementation(s) you should get yourself added to
MAINTAINERSfor the relevant files.
It touches the MPI lib, though, so it also potentially affects RSA and stuff, which is why I introduced it to the kernel originally.
David
Quoted 7 lines · David Howells
Quoted 4 lines · Eric Biggers
This probably should be grouped with my series "crypto: dh - input validation fixes", as this is also a fix for Diffie-Hellman. I was actually expecting Herbert Xu to take these patches, as Diffie-Hellman is now part of the crypto API (
crypto/dh.c); none of the patches actually touchsecurity/keys/.
Quoted 8 lines
Fine by me.
Quoted 2 lines
If you'd like to maintain the Diffie-Hellman implementation(s) you should get yourself added to
MAINTAINERSfor the relevant files.It touches the MPI lib, though, so it also potentially affects RSA and stuff, which is why I introduced it to the kernel originally.
True, mpi_powm() might also be reachable by adding a key of type "asymmetric", in which case this fix would need to go to older kernels than v4.12. If I have a chance I'll see if I can find a reproducer.
Eric
Quoted 13 lines · Eric Biggers
Quoted 12 lines · David Howells
Quoted 4 lines · Eric Biggers
This probably should be grouped with my series "crypto: dh - input validation fixes", as this is also a fix for Diffie-Hellman. I was actually expecting Herbert Xu to take these patches, as Diffie-Hellman is now part of the crypto API (
crypto/dh.c); none of the patches actually touchsecurity/keys/.Fine by me.
Quoted 2 lines
If you'd like to maintain the Diffie-Hellman implementation(s) you should get yourself added to
MAINTAINERSfor the relevant files.
Quoted 7 lines
Quoted 3 lines
It touches the MPI lib, though, so it also potentially affects RSA and stuff, which is why I introduced it to the kernel originally.
True,
mpi_powm()might also be reachable by adding a key of type "asymmetric", in which case this fix would need to go to older kernels than v4.12. If I have a chance I'll see if I can find a reproducer.
CONFIG_KEY_DH_OPERATIONS and use of mpi_powm() by KEYCTL_DH_COMPUTE goes back to v4.7, when the MPI library was called directly. KPP was not implemented yet.
--
Mat Martineau
Intel OTC