* [Bug report] kernel BUG at include/linux/scatterlist.h
@ 2024-11-22 4:51 Zorro Lang
2024-11-22 6:42 ` Ard Biesheuvel
2024-11-29 9:53 ` [PATCH] crypto: rsassa-pkcs1 - Copy source data for SG list Herbert Xu
0 siblings, 2 replies; 14+ messages in thread
From: Zorro Lang @ 2024-11-22 4:51 UTC (permalink / raw)
To: linux-crypto; +Cc: linux-kernel, Herbert Xu
Hi,
I hit a kernel panic on aarch64 several times recently, when I tried to do a
fstests test. It's not related with fstests, due to I hit it when I boot the
latest mainline linux kernel (HEAD=fc39fb56917bb3cb53e99560ca3612a84456ada2).
The console log looks like related with crypto things, I'm not familar with
it, so just send this email to linux-crypto@ and cc linux-kernel@.
I hit this panic several times, I did nothing except building and installing
the latest kernel and then boot it, then it crash directly on booting time.
Looks like crash from:
183 static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
184 unsigned int buflen)
185 {
186 #ifdef CONFIG_DEBUG_SG
==> 187 BUG_ON(!virt_addr_valid(buf));
188 #endif
189 sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf));
190 }
If someone need, I can provide the big linux/.config file.
Thanks,
Zorro
[1]
...
[ 7.313015] registered taskstats version 1
[ 7.320132] Loading compiled-in X.509 certificates
[ 7.347635] Loaded X.509 cert 'Build time autogenerated kernel key: ed2b2ec16b583dda991830c146172ef4fd4cd1cf'
[ 7.522429] Demotion targets for Node 0: null
[ 7.523941] debug_vm_pgtable: [debug_vm_pgtable ]: Validating architecture page table helpers
[ 7.530102] page_owner is disabled
[ 7.532083] Key type .fscrypt registered
[ 7.533070] Key type fscrypt-provisioning registered
[ 7.535734] Key type big_key registered
[ 7.539305] Key type encrypted registered
[ 7.541143] ima: secureboot mode disabled
[ 7.542177] ima: No TPM chip found, activating TPM-bypass!
[ 7.543508] Loading compiled-in module X.509 certificates
[ 7.546783] Loaded X.509 cert 'Build time autogenerated kernel key: ed2b2ec16b583dda991830c146172ef4fd4cd1cf'
[ 7.549427] ima: Allocated hash algorithm: sha256
[ 7.550827] ima: No architecture policies found
[ 7.552706] evm: Initialising EVM extended attributes:
[ 7.554011] evm: security.selinux
[ 7.554842] evm: security.SMACK64 (disabled)
[ 7.555877] evm: security.SMACK64EXEC (disabled)
[ 7.557025] evm: security.SMACK64TRANSMUTE (disabled)
[ 7.558367] evm: security.SMACK64MMAP (disabled)
[ 7.559696] evm: security.apparmor (disabled)
[ 7.560726] evm: security.ima
[ 7.561451] evm: security.capability
[ 7.562297] evm: HMAC attrs: 0x1
[ 7.631769] Running certificate verification RSA selftest
[ 7.652546] ------------[ cut here ]------------
[ 7.653656] kernel BUG at include/linux/scatterlist.h:187!
[ 7.654975] Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
[ 7.656386] Modules linked in:
[ 7.657104] CPU: 3 UID: 0 PID: 176 Comm: cryptomgr_test Not tainted 6.12.0+ #1
[ 7.658822] Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
[ 7.660575] pstate: 20400005 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 7.662385] pc : sg_init_one+0x124/0x150
[ 7.663401] lr : sg_init_one+0x34/0x150
[ 7.664380] sp : ffff800083e87950
[ 7.665240] x29: ffff800083e87950 x28: 0000000000000048 x27: ffff3fc6f3aabc00
[ 7.667078] x26: ffff3fc6f3aabc48 x25: ffff800083e87a60 x24: 1fffe7f8de689885
[ 7.668895] x23: ffffb3207fb10fe0 x22: 0000000000000100 x21: ffffb3207fb7d880
[ 7.670774] x20: ffff800083e87a20 x19: 0000b3207fb7d880 x18: 0000000000000000
[ 7.672568] x17: ffffb3207de83630 x16: ffffb3207de8306c x15: ffffb3207de78cb0
[ 7.674370] x14: ffffb3207d916474 x13: ffffb3207cf38c70 x12: ffff7000107d0f48
[ 7.676156] x11: 1ffff000107d0f47 x10: ffff7000107d0f47 x9 : dfff800000000000
[ 7.677939] x8 : ffff800083e87a40 x7 : 0000000000000000 x6 : 0000000000000004
[ 7.679738] x5 : ffff800083e87a20 x4 : 0000000000000000 x3 : 1ffff000107d0f44
[ 7.681515] x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000600000000000
[ 7.683305] Call trace:
[ 7.683926] sg_init_one+0x124/0x150 (P)
[ 7.684919] sg_init_one+0x34/0x150 (L)
[ 7.685890] rsassa_pkcs1_verify+0x288/0x980
[ 7.686976] test_sig_one+0x344/0x848
[ 7.687916] alg_test_sig+0xc0/0x168
[ 7.688859] alg_test+0x2e0/0xd58
[ 7.689715] cryptomgr_test+0x58/0x88
[ 7.690702] kthread+0x270/0x2f8
[ 7.691542] ret_from_fork+0x10/0x20
[ 7.692470] Code: a8c47bfd d50323bf d65f03c0 d4210000 (d4210000)
[ 7.694042] ---[ end trace 0000000000000000 ]---
[ 7.695246] Kernel panic - not syncing: Oops - BUG: Fatal exception
[ 7.696819] SMP: stopping secondary CPUs
[ 7.697868] Kernel Offset: 0x331ffcf20000 from 0xffff800080000000
[ 7.699457] PHYS_OFFSET: 0xffffc03ac0000000
[ 7.700505] CPU features: 0x00,40000045,00801240,82004203
[ 7.701867] Memory Limit: none
[ 7.702637] ---[ end Kernel panic - not syncing: Oops - BUG: Fatal exception ]---
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bug report] kernel BUG at include/linux/scatterlist.h
2024-11-22 4:51 [Bug report] kernel BUG at include/linux/scatterlist.h Zorro Lang
@ 2024-11-22 6:42 ` Ard Biesheuvel
2024-11-22 7:44 ` Herbert Xu
2024-11-29 9:53 ` [PATCH] crypto: rsassa-pkcs1 - Copy source data for SG list Herbert Xu
1 sibling, 1 reply; 14+ messages in thread
From: Ard Biesheuvel @ 2024-11-22 6:42 UTC (permalink / raw)
To: Zorro Lang; +Cc: linux-crypto, linux-kernel, Herbert Xu
On Fri, 22 Nov 2024 at 05:51, Zorro Lang <zlang@redhat.com> wrote:
>
> Hi,
>
> I hit a kernel panic on aarch64 several times recently, when I tried to do a
> fstests test. It's not related with fstests, due to I hit it when I boot the
> latest mainline linux kernel (HEAD=fc39fb56917bb3cb53e99560ca3612a84456ada2).
>
> The console log looks like related with crypto things, I'm not familar with
> it, so just send this email to linux-crypto@ and cc linux-kernel@.
>
> I hit this panic several times, I did nothing except building and installing
> the latest kernel and then boot it, then it crash directly on booting time.
> Looks like crash from:
>
> 183 static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
> 184 unsigned int buflen)
> 185 {
> 186 #ifdef CONFIG_DEBUG_SG
> ==> 187 BUG_ON(!virt_addr_valid(buf));
> 188 #endif
> 189 sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf));
> 190 }
>
> If someone need, I can provide the big linux/.config file.
>
Does this help?
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -4300,12 +4300,14 @@
static int test_sig_one(struct crypto_sig *tfm, const struct sig_testvec *vecs)
{
+ const u8 *src __free(kfree);
u8 *ptr, *key __free(kfree);
int err, sig_size;
+ src = kmemdup_nul(vecs->c, vecs->c_size, GFP_KERNEL);
key = kmalloc(vecs->key_len + 2 * sizeof(u32) + vecs->param_len,
GFP_KERNEL);
- if (!key)
+ if (!src || !key)
return -ENOMEM;
/* ecrdsa expects additional parameters appended to the key */
@@ -4326,7 +4328,7 @@
* Run asymmetric signature verification first
* (which does not require a private key)
*/
- err = crypto_sig_verify(tfm, vecs->c, vecs->c_size,
+ err = crypto_sig_verify(tfm, src, vecs->c_size,
vecs->m, vecs->m_size);
if (err) {
pr_err("alg: sig: verify test failed: err %d\n", err);
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bug report] kernel BUG at include/linux/scatterlist.h
2024-11-22 6:42 ` Ard Biesheuvel
@ 2024-11-22 7:44 ` Herbert Xu
2024-11-22 8:37 ` Herbert Xu
2024-11-22 19:28 ` Zorro Lang
0 siblings, 2 replies; 14+ messages in thread
From: Herbert Xu @ 2024-11-22 7:44 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Zorro Lang, linux-crypto, linux-kernel
On Fri, Nov 22, 2024 at 07:42:54AM +0100, Ard Biesheuvel wrote:
>
> Does this help?
This is a bug in the API/driver. Users should not be expected to know
what kind of a virtual pointer is acceptable.
In this particular case, rsassa-pkcs1.c should be fixed to use the
crypto_akcipher_sync_encrypt interface.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bug report] kernel BUG at include/linux/scatterlist.h
2024-11-22 7:44 ` Herbert Xu
@ 2024-11-22 8:37 ` Herbert Xu
2024-11-24 16:24 ` Lukas Wunner
2024-11-22 19:28 ` Zorro Lang
1 sibling, 1 reply; 14+ messages in thread
From: Herbert Xu @ 2024-11-22 8:37 UTC (permalink / raw)
To: Ard Biesheuvel, Lukas Wunner; +Cc: Zorro Lang, linux-crypto, linux-kernel
On Fri, Nov 22, 2024 at 03:44:27PM +0800, Herbert Xu wrote:
>
> This is a bug in the API/driver. Users should not be expected to know
> what kind of a virtual pointer is acceptable.
>
> In this particular case, rsassa-pkcs1.c should be fixed to use the
> crypto_akcipher_sync_encrypt interface.
Lukas, could you please take a look at this regression?
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bug report] kernel BUG at include/linux/scatterlist.h
2024-11-22 7:44 ` Herbert Xu
2024-11-22 8:37 ` Herbert Xu
@ 2024-11-22 19:28 ` Zorro Lang
1 sibling, 0 replies; 14+ messages in thread
From: Zorro Lang @ 2024-11-22 19:28 UTC (permalink / raw)
To: Herbert Xu, Ard Biesheuvel; +Cc: linux-crypto, linux-kernel
On Fri, Nov 22, 2024 at 03:44:27PM +0800, Herbert Xu wrote:
> On Fri, Nov 22, 2024 at 07:42:54AM +0100, Ard Biesheuvel wrote:
> >
> > Does this help?
Hi Ard, thanks for your quick response, do you still hope to test
your patch?
>
> This is a bug in the API/driver. Users should not be expected to know
> what kind of a virtual pointer is acceptable.
>
> In this particular case, rsassa-pkcs1.c should be fixed to use the
> crypto_akcipher_sync_encrypt interface.
Thanks Herbert, feel free to CC me after you have a patch, I'm glad to
give it a test.
Thanks,
Zorro
>
> Thanks,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bug report] kernel BUG at include/linux/scatterlist.h
2024-11-22 8:37 ` Herbert Xu
@ 2024-11-24 16:24 ` Lukas Wunner
2024-11-24 23:13 ` Herbert Xu
0 siblings, 1 reply; 14+ messages in thread
From: Lukas Wunner @ 2024-11-24 16:24 UTC (permalink / raw)
To: Herbert Xu; +Cc: Ard Biesheuvel, Zorro Lang, linux-crypto, linux-kernel
On Fri, Nov 22, 2024 at 04:37:09PM +0800, Herbert Xu wrote:
> On Fri, Nov 22, 2024 at 03:44:27PM +0800, Herbert Xu wrote:
> >
> > This is a bug in the API/driver. Users should not be expected to know
> > what kind of a virtual pointer is acceptable.
> >
> > In this particular case, rsassa-pkcs1.c should be fixed to use the
> > crypto_akcipher_sync_encrypt interface.
>
> Lukas, could you please take a look at this regression?
Hm, my impression is that this needs to be fixed in arm64's
virt_addr_valid() macro.
Let's see what arm64 maintainers have to say about it:
https://lore.kernel.org/r/90667b2b7f773308318261f96ebefd1a67133c4c.1732464395.git.lukas@wunner.de/
Thanks,
Lukas
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bug report] kernel BUG at include/linux/scatterlist.h
2024-11-24 16:24 ` Lukas Wunner
@ 2024-11-24 23:13 ` Herbert Xu
2024-11-25 10:29 ` Lukas Wunner
0 siblings, 1 reply; 14+ messages in thread
From: Herbert Xu @ 2024-11-24 23:13 UTC (permalink / raw)
To: Lukas Wunner; +Cc: Ard Biesheuvel, Zorro Lang, linux-crypto, linux-kernel
On Sun, Nov 24, 2024 at 05:24:12PM +0100, Lukas Wunner wrote:
>
> Hm, my impression is that this needs to be fixed in arm64's
> virt_addr_valid() macro.
Regardless of what happens on arm64, you can't put a virtual
address into an SG list in general. It's just not allowed.
In any case, we don't even need SG lists here since the correct
interface to use in rsassa-pkcs1.c is crypto_akcipher_sync_encrypt.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bug report] kernel BUG at include/linux/scatterlist.h
2024-11-24 23:13 ` Herbert Xu
@ 2024-11-25 10:29 ` Lukas Wunner
2024-11-25 10:37 ` Herbert Xu
0 siblings, 1 reply; 14+ messages in thread
From: Lukas Wunner @ 2024-11-25 10:29 UTC (permalink / raw)
To: Herbert Xu; +Cc: Ard Biesheuvel, Zorro Lang, linux-crypto, linux-kernel
On Mon, Nov 25, 2024 at 07:13:18AM +0800, Herbert Xu wrote:
> On Sun, Nov 24, 2024 at 05:24:12PM +0100, Lukas Wunner wrote:
> > Hm, my impression is that this needs to be fixed in arm64's
> > virt_addr_valid() macro.
>
> Regardless of what happens on arm64, you can't put a virtual
> address into an SG list in general. It's just not allowed.
The virtual address passed to sg_init_one() is converted to a
physical address with the following call chain:
sg_init_one()
sg_set_buf()
sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf))
... where virt_to_page() implicitly does the right thing for
kmalloc'ed addresses ("linear map") versus kernel image addresses
on arm64 (as on other arches):
virt_to_page()
pfn_to_page(virt_to_pfn())
__phys_to_pfn(virt_to_phys())
__virt_to_phys()
__virt_to_phys_nodebug()
__is_lm_address(__x) ? __lm_to_phys(__x) : __kimg_to_phys(__x)
So this all works fine and with the patch I proposed, all crypto
selftests pass in my qemu arm64 VM.
It's just that the virt_addr_valid() check in sg_set_buf() throws
a nonsensical false-positive BUG_ON() on arm64.
> In any case, we don't even need SG lists here since the correct
> interface to use in rsassa-pkcs1.c is crypto_akcipher_sync_encrypt.
crypto_akcipher_sync_encrypt() kmalloc's a buffer and copies from
the kernel's .rodata section to that buffer. That's why it doesn't
throw the false-positive BUG_ON() on arm64: virt_addr_valid() is
happy if the virtual address is in the linear map.
Nevertheless, crypto_akcipher_sync_encrypt() likewise passes a virtual
address to sg_init_one(), which is converted to a physical address
in the linear map as shown above.
I deliberately avoided the crypto_akcipher_sync_encrypt() API
in rsassa-pkcs1.c because the extra buffer allocation plus copying
data around impacts performance for no benefit.
There is a benefit of course in that the false-positive BUG_ON()
isn't triggered but that's an arm64 oddity that other major arches
do not exhibit and that should be fixed.
So if you absolutely positively want to use crypto_akcipher_sync_encrypt()
in rsassa-pkcs1.c, I can change that. But it will come at a performance
cost without apparent benefit. Are you sure (y/n)?
Thanks,
Lukas
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bug report] kernel BUG at include/linux/scatterlist.h
2024-11-25 10:29 ` Lukas Wunner
@ 2024-11-25 10:37 ` Herbert Xu
2024-11-29 7:54 ` Lukas Wunner
0 siblings, 1 reply; 14+ messages in thread
From: Herbert Xu @ 2024-11-25 10:37 UTC (permalink / raw)
To: Lukas Wunner; +Cc: Ard Biesheuvel, Zorro Lang, linux-crypto, linux-kernel
On Mon, Nov 25, 2024 at 11:29:30AM +0100, Lukas Wunner wrote:
>
> crypto_akcipher_sync_encrypt() kmalloc's a buffer and copies from
> the kernel's .rodata section to that buffer. That's why it doesn't
> throw the false-positive BUG_ON() on arm64: virt_addr_valid() is
> happy if the virtual address is in the linear map.
That's the whole point, only kmalloced addresses are allowed for
SG lists. You cannot place an arbitrary virtual address into an
SG list, it's just broken.
> I deliberately avoided the crypto_akcipher_sync_encrypt() API
> in rsassa-pkcs1.c because the extra buffer allocation plus copying
> data around impacts performance for no benefit.
This is temporary. The idea is to convert the akcipher software
implementations over to use virtual addresses directly so that no
unnecessary copy occurs. Have a look at what I did with ahash:
https://lore.kernel.org/linux-crypto/bffef4bab1bf250bd64a3d02de53eb1fd047a96e.1730021644.git.herbert@gondor.apana.org.au/
This is what I'd like to do with akcipher as well.
Longer term there is potentially another unnecessary copy if you
go from a kmalloced virtual address to an akcipher hardware driver,
but we could eliminate that by adding a flag to indicate that the
virtual address is safe for use within an SG list.
> So if you absolutely positively want to use crypto_akcipher_sync_encrypt()
> in rsassa-pkcs1.c, I can change that. But it will come at a performance
> cost without apparent benefit. Are you sure (y/n)?
Yes.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bug report] kernel BUG at include/linux/scatterlist.h
2024-11-25 10:37 ` Herbert Xu
@ 2024-11-29 7:54 ` Lukas Wunner
2024-11-29 8:03 ` Herbert Xu
0 siblings, 1 reply; 14+ messages in thread
From: Lukas Wunner @ 2024-11-29 7:54 UTC (permalink / raw)
To: Herbert Xu; +Cc: Ard Biesheuvel, Zorro Lang, linux-crypto, linux-kernel
On Mon, Nov 25, 2024 at 06:37:54PM +0800, Herbert Xu wrote:
> On Mon, Nov 25, 2024 at 11:29:30AM +0100, Lukas Wunner wrote:
> > I deliberately avoided the crypto_akcipher_sync_encrypt() API
> > in rsassa-pkcs1.c because the extra buffer allocation plus copying
> > data around impacts performance for no benefit.
>
> This is temporary. The idea is to convert the akcipher software
> implementations over to use virtual addresses directly so that no
> unnecessary copy occurs. Have a look at what I did with ahash:
>
> https://lore.kernel.org/linux-crypto/bffef4bab1bf250bd64a3d02de53eb1fd047a96e.1730021644.git.herbert@gondor.apana.org.au/
>
> This is what I'd like to do with akcipher as well.
>
> Longer term there is potentially another unnecessary copy if you
> go from a kmalloced virtual address to an akcipher hardware driver,
> but we could eliminate that by adding a flag to indicate that the
> virtual address is safe for use within an SG list.
I'm proposing an optimized approach which auto-detects whether
duplicating into the linear map is actually necessary:
https://lore.kernel.org/r/3de5d373c86dcaa5abc36f501c1398c4fbf05f2f.1732865109.git.lukas@wunner.de/
I reckon that aside of selftests, duplicating is almost never needed.
Let me know if you have any objections. My apologies for being
stubborn and rebellious and not always following what I'm told.
Thanks,
Lukas
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bug report] kernel BUG at include/linux/scatterlist.h
2024-11-29 7:54 ` Lukas Wunner
@ 2024-11-29 8:03 ` Herbert Xu
0 siblings, 0 replies; 14+ messages in thread
From: Herbert Xu @ 2024-11-29 8:03 UTC (permalink / raw)
To: Lukas Wunner; +Cc: Ard Biesheuvel, Zorro Lang, linux-crypto, linux-kernel
On Fri, Nov 29, 2024 at 08:54:35AM +0100, Lukas Wunner wrote:
>
> I'm proposing an optimized approach which auto-detects whether
> duplicating into the linear map is actually necessary:
>
> https://lore.kernel.org/r/3de5d373c86dcaa5abc36f501c1398c4fbf05f2f.1732865109.git.lukas@wunner.de/
No this is not going to work. Linux runs on many platforms,
there is no portable way of doing DMA over arbitrary kernel
memory.
Only memory specifically allocated for DMA is safe to use in
an SG list.
Since you're refusing to fix this I'll take care of it.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] crypto: rsassa-pkcs1 - Copy source data for SG list
2024-11-22 4:51 [Bug report] kernel BUG at include/linux/scatterlist.h Zorro Lang
2024-11-22 6:42 ` Ard Biesheuvel
@ 2024-11-29 9:53 ` Herbert Xu
2024-11-30 8:41 ` Lukas Wunner
1 sibling, 1 reply; 14+ messages in thread
From: Herbert Xu @ 2024-11-29 9:53 UTC (permalink / raw)
To: Zorro Lang
Cc: linux-crypto, linux-kernel, Lukas Wunner, Ard Biesheuvel,
Mark Rutland
As virtual addresses in general may not be suitable for DMA, always
perform a copy before using them in an SG list.
Fixes: 1e562deacecc ("crypto: rsassa-pkcs1 - Migrate to sig_alg backend")
Reported-by: Zorro Lang <zlang@redhat.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/crypto/rsassa-pkcs1.c b/crypto/rsassa-pkcs1.c
index 4d077fc96076..f68ffd338f48 100644
--- a/crypto/rsassa-pkcs1.c
+++ b/crypto/rsassa-pkcs1.c
@@ -163,10 +163,6 @@ static int rsassa_pkcs1_sign(struct crypto_sig *tfm,
struct rsassa_pkcs1_inst_ctx *ictx = sig_instance_ctx(inst);
const struct hash_prefix *hash_prefix = ictx->hash_prefix;
struct rsassa_pkcs1_ctx *ctx = crypto_sig_ctx(tfm);
- unsigned int child_reqsize = crypto_akcipher_reqsize(ctx->child);
- struct akcipher_request *child_req __free(kfree_sensitive) = NULL;
- struct scatterlist in_sg[3], out_sg;
- struct crypto_wait cwait;
unsigned int pad_len;
unsigned int ps_end;
unsigned int len;
@@ -187,37 +183,25 @@ static int rsassa_pkcs1_sign(struct crypto_sig *tfm,
pad_len = ctx->key_size - slen - hash_prefix->size - 1;
- child_req = kmalloc(sizeof(*child_req) + child_reqsize + pad_len,
- GFP_KERNEL);
- if (!child_req)
- return -ENOMEM;
-
/* RFC 8017 sec 8.2.1 step 1 - EMSA-PKCS1-v1_5 encoding generation */
- in_buf = (u8 *)(child_req + 1) + child_reqsize;
+ in_buf = dst;
+ memmove(in_buf + pad_len + hash_prefix->size, src, slen);
+ memcpy(in_buf + pad_len, hash_prefix->data, hash_prefix->size);
+
ps_end = pad_len - 1;
in_buf[0] = 0x01;
memset(in_buf + 1, 0xff, ps_end - 1);
in_buf[ps_end] = 0x00;
- /* RFC 8017 sec 8.2.1 step 2 - RSA signature */
- crypto_init_wait(&cwait);
- sg_init_table(in_sg, 3);
- sg_set_buf(&in_sg[0], in_buf, pad_len);
- sg_set_buf(&in_sg[1], hash_prefix->data, hash_prefix->size);
- sg_set_buf(&in_sg[2], src, slen);
- sg_init_one(&out_sg, dst, dlen);
- akcipher_request_set_tfm(child_req, ctx->child);
- akcipher_request_set_crypt(child_req, in_sg, &out_sg,
- ctx->key_size - 1, dlen);
- akcipher_request_set_callback(child_req, CRYPTO_TFM_REQ_MAY_SLEEP,
- crypto_req_done, &cwait);
- err = crypto_akcipher_decrypt(child_req);
- err = crypto_wait_req(err, &cwait);
- if (err)
+ /* RFC 8017 sec 8.2.1 step 2 - RSA signature */
+ err = crypto_akcipher_sync_decrypt(ctx->child, in_buf,
+ ctx->key_size - 1, in_buf,
+ ctx->key_size);
+ if (err < 0)
return err;
- len = child_req->dst_len;
+ len = err;
pad_len = ctx->key_size - len;
/* Four billion to one */
@@ -239,8 +223,8 @@ static int rsassa_pkcs1_verify(struct crypto_sig *tfm,
struct rsassa_pkcs1_ctx *ctx = crypto_sig_ctx(tfm);
unsigned int child_reqsize = crypto_akcipher_reqsize(ctx->child);
struct akcipher_request *child_req __free(kfree_sensitive) = NULL;
- struct scatterlist in_sg, out_sg;
struct crypto_wait cwait;
+ struct scatterlist sg;
unsigned int dst_len;
unsigned int pos;
u8 *out_buf;
@@ -259,13 +243,12 @@ static int rsassa_pkcs1_verify(struct crypto_sig *tfm,
return -ENOMEM;
out_buf = (u8 *)(child_req + 1) + child_reqsize;
+ memcpy(out_buf, src, slen);
crypto_init_wait(&cwait);
- sg_init_one(&in_sg, src, slen);
- sg_init_one(&out_sg, out_buf, ctx->key_size);
+ sg_init_one(&sg, out_buf, slen);
akcipher_request_set_tfm(child_req, ctx->child);
- akcipher_request_set_crypt(child_req, &in_sg, &out_sg,
- slen, ctx->key_size);
+ akcipher_request_set_crypt(child_req, &sg, &sg, slen, slen);
akcipher_request_set_callback(child_req, CRYPTO_TFM_REQ_MAY_SLEEP,
crypto_req_done, &cwait);
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] crypto: rsassa-pkcs1 - Copy source data for SG list
2024-11-29 9:53 ` [PATCH] crypto: rsassa-pkcs1 - Copy source data for SG list Herbert Xu
@ 2024-11-30 8:41 ` Lukas Wunner
2024-12-03 7:57 ` Herbert Xu
0 siblings, 1 reply; 14+ messages in thread
From: Lukas Wunner @ 2024-11-30 8:41 UTC (permalink / raw)
To: Herbert Xu
Cc: Zorro Lang, linux-crypto, linux-kernel, Ard Biesheuvel,
Mark Rutland
On Fri, Nov 29, 2024 at 05:53:16PM +0800, Herbert Xu wrote:
> As virtual addresses in general may not be suitable for DMA, always
> perform a copy before using them in an SG list.
Just a heads-up, this won't work for use cases when the src buffer
isn't accessible by the kernel. E.g. if the virtual address pointed to
by src is in TEE restricted memory which was mapped into virtual address
space by dma_buf_vmap():
https://lore.kernel.org/all/20241128150927.1377981-1-jens.wiklander@linaro.org/
Thanks,
Lukas
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] crypto: rsassa-pkcs1 - Copy source data for SG list
2024-11-30 8:41 ` Lukas Wunner
@ 2024-12-03 7:57 ` Herbert Xu
0 siblings, 0 replies; 14+ messages in thread
From: Herbert Xu @ 2024-12-03 7:57 UTC (permalink / raw)
To: Lukas Wunner
Cc: Zorro Lang, linux-crypto, linux-kernel, Ard Biesheuvel,
Mark Rutland
On Sat, Nov 30, 2024 at 09:41:40AM +0100, Lukas Wunner wrote:
>
> Just a heads-up, this won't work for use cases when the src buffer
> isn't accessible by the kernel. E.g. if the virtual address pointed to
> by src is in TEE restricted memory which was mapped into virtual address
> space by dma_buf_vmap():
>
> https://lore.kernel.org/all/20241128150927.1377981-1-jens.wiklander@linaro.org/
If it's not accessible by the kernel, why would you map into the
kernel page table? That just makes no sense.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-12-03 7:58 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-22 4:51 [Bug report] kernel BUG at include/linux/scatterlist.h Zorro Lang
2024-11-22 6:42 ` Ard Biesheuvel
2024-11-22 7:44 ` Herbert Xu
2024-11-22 8:37 ` Herbert Xu
2024-11-24 16:24 ` Lukas Wunner
2024-11-24 23:13 ` Herbert Xu
2024-11-25 10:29 ` Lukas Wunner
2024-11-25 10:37 ` Herbert Xu
2024-11-29 7:54 ` Lukas Wunner
2024-11-29 8:03 ` Herbert Xu
2024-11-22 19:28 ` Zorro Lang
2024-11-29 9:53 ` [PATCH] crypto: rsassa-pkcs1 - Copy source data for SG list Herbert Xu
2024-11-30 8:41 ` Lukas Wunner
2024-12-03 7:57 ` Herbert Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).