HIGHMEM pages may not be mapped so we must kmap them before accessing.
This resolves a random OOPs error that was showing up during OpenSSL SHA tests.
Signed-off-by: Joel Fernandes <[email protected]>
Cc: Herbert Xu <[email protected]>
---
v2 changes:
- don't check for HIGHMEM, kmap/kunmap do them anyway (Thanks Herbert).
drivers/crypto/omap-sham.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
index e45aaaf..207eac1 100644
--- a/drivers/crypto/omap-sham.c
+++ b/drivers/crypto/omap-sham.c
@@ -636,11 +636,17 @@ static size_t omap_sham_append_buffer(struct omap_sham_reqctx *ctx,
static size_t omap_sham_append_sg(struct omap_sham_reqctx *ctx)
{
size_t count;
+ const u8 *vaddr;
while (ctx->sg) {
+ vaddr = kmap(sg_page(ctx->sg));
+
count = omap_sham_append_buffer(ctx,
- sg_virt(ctx->sg) + ctx->offset,
+ vaddr + ctx->offset,
ctx->sg->length - ctx->offset);
+
+ kunmap((void *)vaddr);
+
if (!count)
break;
ctx->offset += count;
--
1.7.9.5
On Tue, Mar 04, 2014 at 12:30:54PM -0600, Joel Fernandes wrote:
> HIGHMEM pages may not be mapped so we must kmap them before accessing.
> This resolves a random OOPs error that was showing up during OpenSSL SHA tests.
>
> Signed-off-by: Joel Fernandes <[email protected]>
> Cc: Herbert Xu <[email protected]>
> ---
> v2 changes:
> - don't check for HIGHMEM, kmap/kunmap do them anyway (Thanks Herbert).
>
> drivers/crypto/omap-sham.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
> index e45aaaf..207eac1 100644
> --- a/drivers/crypto/omap-sham.c
> +++ b/drivers/crypto/omap-sham.c
> @@ -636,11 +636,17 @@ static size_t omap_sham_append_buffer(struct omap_sham_reqctx *ctx,
> static size_t omap_sham_append_sg(struct omap_sham_reqctx *ctx)
> {
> size_t count;
> + const u8 *vaddr;
>
> while (ctx->sg) {
> + vaddr = kmap(sg_page(ctx->sg));
Are you sure you can safely use kmap here as opposed to kmap_atomic?
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> On Mar 4, 2014, at 8:00 PM, "Herbert Xu" <[email protected]> wrote:
>
>> On Tue, Mar 04, 2014 at 12:30:54PM -0600, Joel Fernandes wrote:
>> HIGHMEM pages may not be mapped so we must kmap them before accessing.
>> This resolves a random OOPs error that was showing up during OpenSSL SHA tests.
>>
>> Signed-off-by: Joel Fernandes <[email protected]>
>> Cc: Herbert Xu <[email protected]>
>> ---
>> v2 changes:
>> - don't check for HIGHMEM, kmap/kunmap do them anyway (Thanks Herbert).
>>
>> drivers/crypto/omap-sham.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
>> index e45aaaf..207eac1 100644
>> --- a/drivers/crypto/omap-sham.c
>> +++ b/drivers/crypto/omap-sham.c
>> @@ -636,11 +636,17 @@ static size_t omap_sham_append_buffer(struct omap_sham_reqctx *ctx,
>> static size_t omap_sham_append_sg(struct omap_sham_reqctx *ctx)
>> {
>> size_t count;
>> + const u8 *vaddr;
>>
>> while (ctx->sg) {
>> + vaddr = kmap(sg_page(ctx->sg));
>
> Are you sure you can safely use kmap here as opposed to kmap_atomic?
Yes I made sure it is not called in interrupt context and also tested the patch.
Regards,
-Joel
>
> Thanks,
> --
> Email: Herbert Xu <[email protected]>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Wed, Mar 05, 2014 at 04:18:12AM +0000, Fernandes, Joel wrote:
>
>
> > On Mar 4, 2014, at 8:00 PM, "Herbert Xu" <[email protected]> wrote:
> >
> >> On Tue, Mar 04, 2014 at 12:30:54PM -0600, Joel Fernandes wrote:
> >> HIGHMEM pages may not be mapped so we must kmap them before accessing.
> >> This resolves a random OOPs error that was showing up during OpenSSL SHA tests.
> >>
> >> Signed-off-by: Joel Fernandes <[email protected]>
> >> Cc: Herbert Xu <[email protected]>
> >> ---
> >> v2 changes:
> >> - don't check for HIGHMEM, kmap/kunmap do them anyway (Thanks Herbert).
> >>
> >> drivers/crypto/omap-sham.c | 8 +++++++-
> >> 1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
> >> index e45aaaf..207eac1 100644
> >> --- a/drivers/crypto/omap-sham.c
> >> +++ b/drivers/crypto/omap-sham.c
> >> @@ -636,11 +636,17 @@ static size_t omap_sham_append_buffer(struct omap_sham_reqctx *ctx,
> >> static size_t omap_sham_append_sg(struct omap_sham_reqctx *ctx)
> >> {
> >> size_t count;
> >> + const u8 *vaddr;
> >>
> >> while (ctx->sg) {
> >> + vaddr = kmap(sg_page(ctx->sg));
> >
> > Are you sure you can safely use kmap here as opposed to kmap_atomic?
>
> Yes I made sure it is not called in interrupt context and also tested the patch.
What about omap_sham_update => omap_sham_append_sg? As the former
can be called in softirq context what is preventing it from calling
kmap?
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On 03/05/2014 01:11 AM, Herbert Xu wrote:
> On Wed, Mar 05, 2014 at 04:18:12AM +0000, Fernandes, Joel wrote:
>>
>>
>>> On Mar 4, 2014, at 8:00 PM, "Herbert Xu" <[email protected]> wrote:
>>>
>>>> On Tue, Mar 04, 2014 at 12:30:54PM -0600, Joel Fernandes wrote:
>>>> HIGHMEM pages may not be mapped so we must kmap them before accessing.
>>>> This resolves a random OOPs error that was showing up during OpenSSL SHA tests.
>>>>
>>>> Signed-off-by: Joel Fernandes <[email protected]>
>>>> Cc: Herbert Xu <[email protected]>
>>>> ---
>>>> v2 changes:
>>>> - don't check for HIGHMEM, kmap/kunmap do them anyway (Thanks Herbert).
>>>>
>>>> drivers/crypto/omap-sham.c | 8 +++++++-
>>>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
>>>> index e45aaaf..207eac1 100644
>>>> --- a/drivers/crypto/omap-sham.c
>>>> +++ b/drivers/crypto/omap-sham.c
>>>> @@ -636,11 +636,17 @@ static size_t omap_sham_append_buffer(struct omap_sham_reqctx *ctx,
>>>> static size_t omap_sham_append_sg(struct omap_sham_reqctx *ctx)
>>>> {
>>>> size_t count;
>>>> + const u8 *vaddr;
>>>>
>>>> while (ctx->sg) {
>>>> + vaddr = kmap(sg_page(ctx->sg));
>>>
>>> Are you sure you can safely use kmap here as opposed to kmap_atomic?
>>
>> Yes I made sure it is not called in interrupt context and also tested the patch.
>
> What about omap_sham_update => omap_sham_append_sg? As the former
> can be called in softirq context what is preventing it from calling
> kmap?
In my testing it wasn't called from softirq, I'm using cryptodev which is
called from openssl through ioctl, here is a traceback (sorry about wrap):
[<bf8010bb>] (cryptodev_ioctl+0x282/0x59c [cryptodev])
[ 220.015390] [<bf8010bb>] (cryptodev_ioctl+0x282/0x59c [cryptodev]) from
[<c00cdaa5>] (do_vfs_ioctl+0x61/0x41c)
[ 220.025909] [<c00cdaa5>] (do_vfs_ioctl+0x61/0x41c) from [<c00cdeab>]
(SyS_ioctl+0x4b/0x50)
[ 220.034602] [<c00cdeab>] (SyS_ioctl+0x4b/0x50) from [<c000cf01>]
(ret_fast_syscall+0x1/0x46)
[ 220.045254] CPU: 0 PID: 1798 Comm: openssl Tainted: G O
3.12.9-00581-g5fa084c-dirty #32
[ 220.054636] [<c0012d05>] (unwind_backtrace+0x1/0x9c) from [<c000fa6d>]
(show_stack+0x11/0x14)
[ 220.063619] [<c000fa6d>] (show_stack+0x11/0x14) from [<c0342edb>]
(dump_stack+0x4b/0x60)
[ 220.072144] [<c0342edb>] (dump_stack+0x4b/0x60) from [<c02a1b3f>]
(omap_sham_update+0x1f/0xa4)
[ 220.081214] [<c02a1b3f>] (omap_sham_update+0x1f/0xa4) from [<bf801f67>]
(cryptodev_hash_update+0x26/0x80 [cryptodev])
Could you suggest, what other place is update() called in softirq context?
I may be missing something.
Thanks,
-Joel
>
> Cheers,
>
On Wed, Mar 05, 2014 at 09:42:38AM -0600, Joel Fernandes wrote:
>
> In my testing it wasn't called from softirq, I'm using cryptodev which is
> called from openssl through ioctl, here is a traceback (sorry about wrap):
>
> [<bf8010bb>] (cryptodev_ioctl+0x282/0x59c [cryptodev])
> [ 220.015390] [<bf8010bb>] (cryptodev_ioctl+0x282/0x59c [cryptodev]) from
> [<c00cdaa5>] (do_vfs_ioctl+0x61/0x41c)
> [ 220.025909] [<c00cdaa5>] (do_vfs_ioctl+0x61/0x41c) from [<c00cdeab>]
> (SyS_ioctl+0x4b/0x50)
> [ 220.034602] [<c00cdeab>] (SyS_ioctl+0x4b/0x50) from [<c000cf01>]
> (ret_fast_syscall+0x1/0x46)
> [ 220.045254] CPU: 0 PID: 1798 Comm: openssl Tainted: G O
> 3.12.9-00581-g5fa084c-dirty #32
> [ 220.054636] [<c0012d05>] (unwind_backtrace+0x1/0x9c) from [<c000fa6d>]
> (show_stack+0x11/0x14)
> [ 220.063619] [<c000fa6d>] (show_stack+0x11/0x14) from [<c0342edb>]
> (dump_stack+0x4b/0x60)
> [ 220.072144] [<c0342edb>] (dump_stack+0x4b/0x60) from [<c02a1b3f>]
> (omap_sham_update+0x1f/0xa4)
> [ 220.081214] [<c02a1b3f>] (omap_sham_update+0x1f/0xa4) from [<bf801f67>]
> (cryptodev_hash_update+0x26/0x80 [cryptodev])
>
>
> Could you suggest, what other place is update() called in softirq context?
> I may be missing something.
IPsec may call this from softirq context.
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt