Re: [PATCH] crypto: vmx - Fix sleep-in-atomic bugs
CC: Paulo Flabiano Smorigo ,
linuxppc-dev@lists.ozlabs.org
(Sorry, sent this before reading new e-mails in the thread...)
ut 21. 8. 2018 o 17:18 Ondrej Mosnacek napísal(a):
>
> This patch fixes sleep-in-atomic bugs in AES-CBC and AES-XTS VMX
> implementations. The problem is that the blkcipher_* functions should
> not be called in atomic context.
>
> The bugs can be reproduced via the AF_ALG interface by trying to
> encrypt/decrypt sufficiently large buffers (at least 64 KiB) using the
> VMX implementations of 'cbc(aes)' or 'xts(aes)'. Such operations then
> trigger BUG in crypto_yield():
>
> [ 891.863680] BUG: sleeping function called from invalid context at
> include/crypto/algapi.h:424
> [ 891.864622] in_atomic(): 1, irqs_disabled(): 0, pid: 12347, name: kcapi-enc
> [ 891.864739] 1 lock held by kcapi-enc/12347:
> [ 891.864811] #0: f5d42c46 (sk_lock-AF_ALG){+.+.}, at:
> skcipher_recvmsg+0x50/0x530
> [ 891.865076] CPU: 5 PID: 12347 Comm: kcapi-enc Not tainted
> 4.19.0-0.rc0.git3.1.fc30.ppc64le #1
> [ 891.865251] Call Trace:
> [ 891.865340] [c003387578c0] [c0d67ea4] dump_stack+0xe8/0x164
> (unreliable)
> [ 891.865511] [c00338757910] [c0172a58]
> ___might_sleep+0x2f8/0x310
> [ 891.865679] [c00338757990] [c06bff74]
> blkcipher_walk_done+0x374/0x4a0
> [ 891.865825] [c003387579e0] [d7e73e70]
> p8_aes_cbc_encrypt+0x1c8/0x260 [vmx_crypto]
> [ 891.865993] [c00338757ad0] [c06c0ee0]
> skcipher_encrypt_blkcipher+0x60/0x80
> [ 891.866128] [c00338757b10] [c06ec504]
> skcipher_recvmsg+0x424/0x530
> [ 891.866283] [c00338757bd0] [c0b00654] sock_recvmsg+0x74/0xa0
> [ 891.866403] [c00338757c10] [c0b00f64] ___sys_recvmsg+0xf4/0x2f0
> [ 891.866515] [c00338757d90] [c0b02bb8] __sys_recvmsg+0x68/0xe0
> [ 891.866631] [c00338757e30] [c000bbe4] system_call+0x5c/0x70
>
> Fixes: 8c755ace357c ("crypto: vmx - Adding CBC routines for VMX module")
> Fixes: c07f5d3da643 ("crypto: vmx - Adding support for XTS")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Ondrej Mosnacek
> ---
> This patch should fix the issue, but I didn't test it. (I'll see if I
> can find some time tomorrow to try and recompile the kernel on a PPC
> machine... in the meantime please review :)
>
> drivers/crypto/vmx/aes_cbc.c | 30 ++
> drivers/crypto/vmx/aes_xts.c | 19 ---
> 2 files changed, 26 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/crypto/vmx/aes_cbc.c b/drivers/crypto/vmx/aes_cbc.c
> index 5285ece4f33a..b71895871be3 100644
> --- a/drivers/crypto/vmx/aes_cbc.c
> +++ b/drivers/crypto/vmx/aes_cbc.c
> @@ -107,24 +107,23 @@ static int p8_aes_cbc_encrypt(struct blkcipher_desc
> *desc,
> ret = crypto_skcipher_encrypt(req);
> skcipher_request_zero(req);
> } else {
> - preempt_disable();
> - pagefault_disable();
> - enable_kernel_vsx();
> -
> blkcipher_walk_init(&walk, dst, src, nbytes);
> ret = blkcipher_walk_virt(desc, &walk);
> while ((nbytes = walk.nbytes)) {
> + preempt_disable();
> + pagefault_disable();
> + enable_kernel_vsx();
> aes_p8_cbc_encrypt(walk.src.virt.addr,
>walk.dst.virt.addr,
>nbytes & AES_BLOCK_MASK,
>&ctx->enc_key, walk.iv, 1);
> + disable_kernel_vsx();
> + pagefault_enable();
> + preempt_enable();
> +
> nbytes &= AES_BLOCK_SIZE - 1;
> ret = blkcipher_walk_done(desc, &walk, nbytes);
> }
> -
> - disable_kernel_vsx();
> - pagefault_enable();
> - preempt_enable();
> }
>
> return ret;
> @@ -147,24 +146,23 @@ static int p8_aes_cbc_decrypt(struct blkcipher_desc
> *desc,
> ret = crypto_skcipher_decrypt(req);
> skcipher_request_zero(req);
> } else {
> - preempt_disable();
> - pagefault_disable();
> - enable_kernel_vsx();
> -
> blkcipher_walk_init(&walk, dst, src, nbytes);
> ret = blkcipher_walk_virt(desc, &walk);
> while ((nbytes = walk.nbytes)) {
> + preempt_disable();
> + pagefault_disable();
> + enable_kernel_vsx();
> aes_p8_cbc_encrypt(walk.src.virt.addr,
>walk.dst.virt.addr,
>nbytes & AES_BLOCK_MASK,
>&ctx->dec_key, walk.iv, 0);
> + disable_
Re: [PATCH] crypto: vmx - Fix sleep-in-atomic bugs
ut 21. 8. 2018 o 18:41 Marcelo Henrique Cerri
napísal(a):
> On Tue, Aug 21, 2018 at 05:24:45PM +0200, Ondrej Mosnáček wrote:
> > CC: Paulo Flabiano Smorigo ,
> > linuxppc-dev@lists.ozlabs.org
> >
> > (Sorry, sent this before reading new e-mails in the thread...)
> >
> > ut 21. 8. 2018 o 17:18 Ondrej Mosnacek napísal(a):
> > >
> > > This patch fixes sleep-in-atomic bugs in AES-CBC and AES-XTS VMX
> > > implementations. The problem is that the blkcipher_* functions should
> > > not be called in atomic context.
> > >
> > > The bugs can be reproduced via the AF_ALG interface by trying to
> > > encrypt/decrypt sufficiently large buffers (at least 64 KiB) using the
> > > VMX implementations of 'cbc(aes)' or 'xts(aes)'. Such operations then
> > > trigger BUG in crypto_yield():
> > >
> > > [ 891.863680] BUG: sleeping function called from invalid context at
> > > include/crypto/algapi.h:424
> > > [ 891.864622] in_atomic(): 1, irqs_disabled(): 0, pid: 12347, name:
> > > kcapi-enc
> > > [ 891.864739] 1 lock held by kcapi-enc/12347:
> > > [ 891.864811] #0: f5d42c46 (sk_lock-AF_ALG){+.+.}, at:
> > > skcipher_recvmsg+0x50/0x530
> > > [ 891.865076] CPU: 5 PID: 12347 Comm: kcapi-enc Not tainted
> > > 4.19.0-0.rc0.git3.1.fc30.ppc64le #1
> > > [ 891.865251] Call Trace:
> > > [ 891.865340] [c003387578c0] [c0d67ea4]
> > > dump_stack+0xe8/0x164 (unreliable)
> > > [ 891.865511] [c00338757910] [c0172a58]
> > > ___might_sleep+0x2f8/0x310
> > > [ 891.865679] [c00338757990] [c06bff74]
> > > blkcipher_walk_done+0x374/0x4a0
> > > [ 891.865825] [c003387579e0] [d7e73e70]
> > > p8_aes_cbc_encrypt+0x1c8/0x260 [vmx_crypto]
> > > [ 891.865993] [c00338757ad0] [c06c0ee0]
> > > skcipher_encrypt_blkcipher+0x60/0x80
> > > [ 891.866128] [c00338757b10] [c06ec504]
> > > skcipher_recvmsg+0x424/0x530
> > > [ 891.866283] [c00338757bd0] [c0b00654]
> > > sock_recvmsg+0x74/0xa0
> > > [ 891.866403] [c00338757c10] [c0b00f64]
> > > ___sys_recvmsg+0xf4/0x2f0
> > > [ 891.866515] [c00338757d90] [c0b02bb8]
> > > __sys_recvmsg+0x68/0xe0
> > > [ 891.866631] [c00338757e30] [c000bbe4] system_call+0x5c/0x70
> > >
> > > Fixes: 8c755ace357c ("crypto: vmx - Adding CBC routines for VMX module")
> > > Fixes: c07f5d3da643 ("crypto: vmx - Adding support for XTS")
> > > Cc: sta...@vger.kernel.org
> > > Signed-off-by: Ondrej Mosnacek
> > > ---
> > > This patch should fix the issue, but I didn't test it. (I'll see if I
> > > can find some time tomorrow to try and recompile the kernel on a PPC
> > > machine... in the meantime please review :)
> > >
> > > drivers/crypto/vmx/aes_cbc.c | 30 ++
> > > drivers/crypto/vmx/aes_xts.c | 19 ---
> > > 2 files changed, 26 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/drivers/crypto/vmx/aes_cbc.c b/drivers/crypto/vmx/aes_cbc.c
> > > index 5285ece4f33a..b71895871be3 100644
> > > --- a/drivers/crypto/vmx/aes_cbc.c
> > > +++ b/drivers/crypto/vmx/aes_cbc.c
> > > @@ -107,24 +107,23 @@ static int p8_aes_cbc_encrypt(struct blkcipher_desc
> > > *desc,
> > > ret = crypto_skcipher_encrypt(req);
> > > skcipher_request_zero(req);
> > > } else {
> > > - preempt_disable();
> > > - pagefault_disable();
> > > - enable_kernel_vsx();
> > > -
> > > blkcipher_walk_init(&walk, dst, src, nbytes);
> > > ret = blkcipher_walk_virt(desc, &walk);
> > > while ((nbytes = walk.nbytes)) {
> > > + preempt_disable();
> > > + pagefault_disable();
> > > + enable_kernel_vsx();
> > > aes_p8_cbc_encrypt(walk.src.virt.addr,
> > >walk.dst.virt.addr,
> > >nbytes & AES_BLOCK_MASK,
> > >&ctx->enc_key, walk.iv, 1);
> > > + disable_kernel_vsx();
> > > + pagefault_enable();
> &g
Re: [PATCH] powerpc64/ftrace: Fix ftrace for clang builds
On Tue, Aug 9, 2022 at 11:59 AM Naveen N. Rao
wrote:
> Clang doesn't support -mprofile-kernel ABI, so guard the checks against
> CONFIG_DYNAMIC_FTRACE_WITH_REGS, rather than the elf ABI version.
>
> Fixes: 23b44fc248f420 ("powerpc/ftrace: Make __ftrace_make_{nop/call}()
> common to PPC32 and PPC64")
> Reported-by: Nick Desaulniers
> Reported-by: Ondrej Mosnacek
> Signed-off-by: Naveen N. Rao
> ---
> arch/powerpc/kernel/trace/ftrace.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
With this patch my reproducer [1] is passing, thanks!
Tested-by: Ondrej Mosnacek
[1] https://github.com/ClangBuiltLinux/linux/issues/1682#issue-1330483247
BUG: p8_aes_ctr randomly returns wrong results
Hi,
FYI, the p8_aes_ctr crypto driver (drivers/crypto/vmx/aes_ctr.c) seems
to be seriously broken. When I do repeated encryption using libkcapi
multiple times in a row, I sometimes get a wrong result. This happens
more often with long messages (e.g. at 16 KiB it already happens very
frequently).
To reproduce:
1. Install or locally build libkcapi [1] (you will need the kcapi-enc
binary in PATH) on a ppc64le system.
2. Run the following in bash:
for i in {1..100}; do head -c $((16*1024)) /dev/zero | kcapi-enc -e -c
'ctr(aes)' -p test -s test --pbkdfiter 1 2>/dev/null | sha256sum; done
| sort -u
Expected result:
All invocations produce output with identical checksum.
Actual result:
Multiple different checksums are produced.
When I run 'rmmod vmx_crypto' before running the reproducer, I get
only one (correct) checksum, so this is definitely a bug in the
driver. Other ciphers (cbc(aes), xts(aes)) are not affected, even
though the glue code is very similar. That leads me to believe the
problem is somewhere in the assembly code.
[1] http://github.com/smuellerDD/libkcapi
Cheers,
Ondrej
Re: BUG: p8_aes_ctr randomly returns wrong results
st 13. 3. 2019 o 13:37 Ondrej Mosnáček napísal(a):
> Hi,
>
> FYI, the p8_aes_ctr crypto driver (drivers/crypto/vmx/aes_ctr.c) seems
> to be seriously broken. When I do repeated encryption using libkcapi
> multiple times in a row, I sometimes get a wrong result. This happens
> more often with long messages (e.g. at 16 KiB it already happens very
> frequently).
>
> To reproduce:
> 1. Install or locally build libkcapi [1] (you will need the kcapi-enc
> binary in PATH) on a ppc64le system.
> 2. Run the following in bash:
> for i in {1..100}; do head -c $((16*1024)) /dev/zero | kcapi-enc -e -c
> 'ctr(aes)' -p test -s test --pbkdfiter 1 2>/dev/null | sha256sum; done
> | sort -u
>
> Expected result:
> All invocations produce output with identical checksum.
>
> Actual result:
> Multiple different checksums are produced.
>
> When I run 'rmmod vmx_crypto' before running the reproducer, I get
> only one (correct) checksum, so this is definitely a bug in the
> driver. Other ciphers (cbc(aes), xts(aes)) are not affected, even
> though the glue code is very similar. That leads me to believe the
> problem is somewhere in the assembly code.
>
> [1] http://github.com/smuellerDD/libkcapi
>
> Cheers,
> Ondrej
(Ah, forgot to compare email addresses with MAINTAINERS... let me try these)
Re: [PATCH] crypto: vmx - fix copy-paste error in CTR mode
Hi Daniel,
pi 15. 3. 2019 o 3:09 Daniel Axtens napísal(a):
> The original assembly imported from OpenSSL has two copy-paste
> errors in handling CTR mode. When dealing with a 2 or 3 block tail,
> the code branches to the CBC decryption exit path, rather than to
> the CTR exit path.
>
> This leads to corruption of the IV, which leads to subsequent blocks
> being corrupted.
>
> This can be detected with libkcapi test suite, which is available at
> https://github.com/smuellerDD/libkcapi
>
> Reported-by: Ondrej Mosnáček
> Fixes: 5c380d623ed3 ("crypto: vmx - Add support for VMS instructions by ASM")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Daniel Axtens
Thank you for looking into this and for posting the patch(es)! I
tested the patch yesterday and I can confirm that it makes the
libkcapi tests/reproducer pass.
Assuming you will want to cover the other failures from the new
testmgr tests by a separate patch:
Tested-by: Ondrej Mosnacek
> ---
> drivers/crypto/vmx/aesp8-ppc.pl | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/vmx/aesp8-ppc.pl b/drivers/crypto/vmx/aesp8-ppc.pl
> index d6a9f63d65ba..de78282b8f44 100644
> --- a/drivers/crypto/vmx/aesp8-ppc.pl
> +++ b/drivers/crypto/vmx/aesp8-ppc.pl
> @@ -1854,7 +1854,7 @@ Lctr32_enc8x_three:
> stvx_u $out1,$x10,$out
> stvx_u $out2,$x20,$out
> addi$out,$out,0x30
> - b Lcbc_dec8x_done
> + b Lctr32_enc8x_done
>
> .align 5
> Lctr32_enc8x_two:
> @@ -1866,7 +1866,7 @@ Lctr32_enc8x_two:
> stvx_u $out0,$x00,$out
> stvx_u $out1,$x10,$out
> addi$out,$out,0x20
> - b Lcbc_dec8x_done
> + b Lctr32_enc8x_done
>
> .align 5
> Lctr32_enc8x_one:
> --
> 2.19.1
>
