Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1451513 > unrolled thread
| Started by | Nicolai Stange <nicstange@gmail.com> |
|---|---|
| First post | 2016-07-27 23:10 +0200 |
| Last post | 2016-07-28 09:50 +0200 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
lib/mpi: BUG: sleeping function called from invalid context on next-20160726 Nicolai Stange <nicstange@gmail.com> - 2016-07-27 23:10 +0200
Re: lib/mpi: BUG: sleeping function called from invalid context on next-20160726 Herbert Xu <herbert@gondor.apana.org.au> - 2016-07-28 07:30 +0200
Re: lib/mpi: BUG: sleeping function called from invalid context on next-20160726 Nicolai Stange <nicstange@gmail.com> - 2016-07-28 09:50 +0200
| From | Nicolai Stange <nicstange@gmail.com> |
|---|---|
| Date | 2016-07-27 23:10 +0200 |
| Subject | lib/mpi: BUG: sleeping function called from invalid context on next-20160726 |
| Message-ID | <rZEfw-4Gp-7@gated-at.bofh.it> |
Hi,
with linux-next-20160726, I get this:
BUG: sleeping function called from invalid context at /mnt/scratch/nic/linux-next/mm/slab.h:388
in_atomic(): 1, irqs_disabled(): 0, pid: 369, name: systemd-udevd
no locks held by systemd-udevd/369.
CPU: 2 PID: 369 Comm: systemd-udevd Not tainted 4.7.0-rc1+ #248
Hardware name: Dell Inc. Latitude E6540/0725FP, BIOS A10 06/26/2014
0000000000000286 00000000899a9b52 ffff88003f53b8f8 ffffffff814472d5
ffff8800c0752680 ffffffff81c557d8 ffff88003f53b920 ffffffff810dfba9
ffffffff81c557d8 0000000000000184 0000000000000000 ffff88003f53b948
Call Trace:
[<ffffffff814472d5>] dump_stack+0x86/0xc1
[<ffffffff810dfba9>] ___might_sleep+0x179/0x230
[<ffffffff810dfca9>] __might_sleep+0x49/0x80
[<ffffffff8125f1f1>] kmem_cache_alloc_trace+0x1d1/0x2e0
[<ffffffff81479b20>] ? mpi_alloc+0x20/0x80
[<ffffffff81479b20>] mpi_alloc+0x20/0x80
[<ffffffff81477475>] mpi_read_raw_from_sgl+0xd5/0x1e0
[<ffffffff813e99f6>] rsa_verify+0x66/0x100
[<ffffffff813ea1be>] pkcs1pad_verify+0xae/0xf0
[<ffffffff81404889>] public_key_verify_signature+0x1f9/0x290
[<ffffffff81404935>] public_key_verify_signature_2+0x15/0x20
[<ffffffff8140458c>] verify_signature+0x3c/0x50
[<ffffffff8140680d>] pkcs7_validate_trust+0x11d/0x230
[<ffffffff811eb132>] verify_pkcs7_signature+0xa2/0x150
[<ffffffff8115747d>] mod_verify_sig+0xdd/0x130
[<ffffffff811541cc>] load_module+0x16c/0x2970
[<ffffffff8128b95b>] ? vfs_read+0x11b/0x130
[<ffffffff81292342>] ? kernel_read_file+0x152/0x170
[<ffffffff81156c66>] SYSC_finit_module+0xe6/0x120
[<ffffffff81156cbe>] SyS_finit_module+0xe/0x10
[<ffffffff81003fe7>] do_syscall_64+0x67/0x190
[<ffffffff8189ab3f>] entry_SYSCALL64_slow_path+0x25/0x25
Reason is 127827b9c295 ("lib/mpi: Do not do sg_virt") which makes
mpi_read_raw_from_sgl() calling mpi_alloc() while having a sg entry
mapped via kmap_atomic() and thus, preemption disabled.
I would have sent a patch, but there is another point which puzzles me
in mpi_read_raw_from_sgl():
[...]
const u8 *buff;
[...]
sg_miter_start(&miter, sgl, ents, SG_MITER_ATOMIC | SG_MITER_FROM_SG);
lzeros = 0;
len = 0;
while (nbytes > 0) {
while (len && !*buff) {
lzeros++;
len--;
buff++;
}
Thus, buff isn't initialized before its first use? Or am I misreading
something here?
Thanks,
Nicolai
[toc] | [next] | [standalone]
| From | Herbert Xu <herbert@gondor.apana.org.au> |
|---|---|
| Date | 2016-07-28 07:30 +0200 |
| Subject | Re: lib/mpi: BUG: sleeping function called from invalid context on next-20160726 |
| Message-ID | <rZM3n-1IS-3@gated-at.bofh.it> |
| In reply to | #1451513 |
On Wed, Jul 27, 2016 at 11:05:05PM +0200, Nicolai Stange wrote:
>
> with linux-next-20160726, I get this:
>
> BUG: sleeping function called from invalid context at /mnt/scratch/nic/linux-next/mm/slab.h:388
Does this patch help?
> I would have sent a patch, but there is another point which puzzles me
> in mpi_read_raw_from_sgl():
>
> [...]
> const u8 *buff;
> [...]
> sg_miter_start(&miter, sgl, ents, SG_MITER_ATOMIC | SG_MITER_FROM_SG);
>
> lzeros = 0;
> len = 0;
> while (nbytes > 0) {
> while (len && !*buff) {
> lzeros++;
> len--;
> buff++;
> }
>
>
> Thus, buff isn't initialized before its first use? Or am I misreading
> something here?
On the first entry len is zero therefore we will go to the end of the
loop and initialise buff. Anyway, it will no longer be as confusing
with this patch applied.
Thanks,
---8<---
Subject: lib/mpi: Fix SG miter leak
In mpi_read_raw_from_sgl we may leak the SG miter resouces after
reading the leading zeroes. This patch fixes this by stopping the
iteration once the leading zeroes have been read.
Fixes: 127827b9c295 ("lib/mpi: Do not do sg_virt")
Reported-by: Nicolai Stange <nicstange@gmail.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/lib/mpi/mpicoder.c b/lib/mpi/mpicoder.c
index c6272ae..5a0f75a 100644
--- a/lib/mpi/mpicoder.c
+++ b/lib/mpi/mpicoder.c
@@ -363,6 +363,9 @@ MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes)
lzeros = 0;
}
+ miter.consumed = lzeros;
+ sg_miter_stop(&miter);
+
nbytes -= lzeros;
nbits = nbytes * 8;
if (nbits > MAX_EXTERN_MPI_BITS) {
@@ -390,7 +393,10 @@ MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes)
z = BYTES_PER_MPI_LIMB - nbytes % BYTES_PER_MPI_LIMB;
z %= BYTES_PER_MPI_LIMB;
- for (;;) {
+ while (sg_miter_next(&miter)) {
+ buff = miter.addr;
+ len = miter.length;
+
for (x = 0; x < len; x++) {
a <<= 8;
a |= *buff++;
@@ -400,12 +406,6 @@ MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes)
}
}
z += x;
-
- if (!sg_miter_next(&miter))
- break;
-
- buff = miter.addr;
- len = miter.length;
}
return val;
--
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
[toc] | [prev] | [next] | [standalone]
| From | Nicolai Stange <nicstange@gmail.com> |
|---|---|
| Date | 2016-07-28 09:50 +0200 |
| Message-ID | <rZOeR-36k-7@gated-at.bofh.it> |
| In reply to | #1451722 |
Herbert Xu <herbert@gondor.apana.org.au> writes:
> On Wed, Jul 27, 2016 at 11:05:05PM +0200, Nicolai Stange wrote:
>>
>> with linux-next-20160726, I get this:
>>
>> BUG: sleeping function called from invalid context at /mnt/scratch/nic/linux-next/mm/slab.h:388
>
> Does this patch help?
Yes, works like a charm now!
>> I would have sent a patch, but there is another point which puzzles me
>> in mpi_read_raw_from_sgl():
>>
>> [...]
>> const u8 *buff;
>> [...]
>> sg_miter_start(&miter, sgl, ents, SG_MITER_ATOMIC | SG_MITER_FROM_SG);
>>
>> lzeros = 0;
>> len = 0;
>> while (nbytes > 0) {
>> while (len && !*buff) {
>> lzeros++;
>> len--;
>> buff++;
>> }
>>
>>
>> Thus, buff isn't initialized before its first use? Or am I misreading
>> something here?
>
> On the first entry len is zero therefore we will go to the end of the
> loop and initialise buff.
Hah! Thanks, although being obvious, I didn't see this...
Thanks,
Nicolai
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web