2004-10-18 19:32:16

by Matt Domsch

[permalink] [raw]
Subject: using crypto_digest() on non-kmalloc'd memory failures

James, David,

Oleg noted that when we call crypto_digest() on memory allocated as a
static array in a module, rather than kmalloc(GFP_KERNEL), it returns
incorrect data, and with other functions, a kernel panic.

Thoughts as to why this may be? Oleg's test patch appended.

--
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions linux.dell.com & http://www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

----- Forwarded message from Oleg Makarenko <[email protected]> -----

From: Oleg Makarenko <[email protected]>
To: Matt Domsch <[email protected]>
CC: [email protected], [email protected]
Subject: Re: [pptp-devel] Re: [2/2]: ppp_mppe inclusion
Date: Sun, 05 Sep 2004 22:23:15 +0400

>>>2. For some reason you can not use non GFP_KERNEL memory and scatter
>>>lists or at least mix them in crypto_digest(). That is why sha_pad is
>>>now in struct state {}.
>
>Can you describe what happens when you do?

please try the attached patch for tcrypt.c to see what is going on
yourself. modprobe the resulting module with mode=2 parameter to test
sha1 and see how it fails the tests (incorrect results, no kernel panic).

For mode=0 (or without any parameter) you should get kernel panic.

=oleg

--- tcrypt.c.orig 2004-08-14 09:37:38.000000000 +0400
+++ tcrypt.c 2004-09-05 21:11:19.000000000 +0400
@@ -58,6 +58,8 @@
static char *xbuf;
static char *tvmem;

+static char tvmem_buf[TVMEMSIZE];
+
static char *check[] = {
"des", "md5", "des3_ede", "rot13", "sha1", "sha256", "blowfish",
"twofish", "serpent", "sha384", "sha512", "md4", "aes", "cast6",
@@ -820,7 +822,8 @@
static int __init
init(void)
{
- tvmem = kmalloc(TVMEMSIZE, GFP_KERNEL);
+ tvmem = &tvmem_buf[0];
+
if (tvmem == NULL)
return -ENOMEM;

@@ -833,7 +836,6 @@
do_test();

kfree(xbuf);
- kfree(tvmem);
return 0;
}


2004-10-18 20:13:24

by James Morris

[permalink] [raw]
Subject: Re: using crypto_digest() on non-kmalloc'd memory failures

On Mon, 18 Oct 2004, Matt Domsch wrote:

> James, David,
>
> Oleg noted that when we call crypto_digest() on memory allocated as a
> static array in a module, rather than kmalloc(GFP_KERNEL), it returns
> incorrect data, and with other functions, a kernel panic.
>
> Thoughts as to why this may be? Oleg's test patch appended.

I don't recall the exact details, but it's related to using kmap in the
core crypto code.


- James
--
James Morris
<[email protected]>


2004-10-18 20:09:51

by Oleg Makarenko

[permalink] [raw]
Subject: Re: using crypto_digest() on non-kmalloc'd memory failures

James Morris wrote:

>On Mon, 18 Oct 2004, Matt Domsch wrote:
>
>
>
>>James, David,
>>
>>Oleg noted that when we call crypto_digest() on memory allocated as a
>>static array in a module, rather than kmalloc(GFP_KERNEL), it returns
>>incorrect data, and with other functions, a kernel panic.
>>
>>Thoughts as to why this may be? Oleg's test patch appended.
>>
>>
>
>I don't recall the exact details, but it's related to using kmap in the
>core crypto code.
>
>
>- James
>
>
So to calculate digest on some static data I need to copy them to
kmalloc'ed memory first, right?

Can this copying be somehow avoided?

And one more question on crypto api. It looks like it is not very
effective for a single byte "block" ciphers as arc4. The overhead is
probably too big. Just look at the loop in cipher.c/crypt() and the code
in arc4.c/arc4_crypt(). All this code is called for every single clear
text byte. Right? Looks like an overkill for bsize == 1.

Is there any better way to use crypto api for arc4 or similar ciphers?
Cipher block size is not always a natural choice for the crypto_yield().
Especially for fast ciphers (arc4) and small "block" sizes (arc4 again).

Or have I missed something obvious?

=oleg

2004-10-18 20:23:44

by David Miller

[permalink] [raw]
Subject: Re: using crypto_digest() on non-kmalloc'd memory failures

On Tue, 19 Oct 2004 00:05:41 +0400
Oleg Makarenko <[email protected]> wrote:

> So to calculate digest on some static data I need to copy them to
> kmalloc'ed memory first, right?
>
> Can this copying be somehow avoided?

It is necessary to be able to kmap() the data item
passed into the crypto layer, so dynamically allocated
memory obtained via kmalloc() or similar must be used.

2004-10-18 20:33:52

by Brian Gerst

[permalink] [raw]
Subject: Re: using crypto_digest() on non-kmalloc'd memory failures

Matt Domsch wrote:
> James, David,
>
> Oleg noted that when we call crypto_digest() on memory allocated as a
> static array in a module, rather than kmalloc(GFP_KERNEL), it returns
> incorrect data, and with other functions, a kernel panic.
>
> Thoughts as to why this may be? Oleg's test patch appended.
>

On some arches modules reside in vmalloc space.

--
Brian Gerst

2004-10-18 22:10:52

by Gene Heskett

[permalink] [raw]
Subject: Re: using crypto_digest() on non-kmalloc'd memory failures

On Monday 18 October 2004 15:29, Matt Domsch wrote:
>James, David,
>
>Oleg noted that when we call crypto_digest() on memory allocated as
> a static array in a module, rather than kmalloc(GFP_KERNEL), it
> returns incorrect data, and with other functions, a kernel panic.
>
>Thoughts as to why this may be? Oleg's test patch appended.

Off topic Matt, but why am I getting 4 to 6 copies of the messages you
send? All identical time stamps, the whole works.

--
Cheers, Gene
"There are four boxes to be used in defense of liberty:
soap, ballot, jury, and ammo. Please use in that order."
-Ed Howdershelt (Author)
99.27% setiathome rank, not too shabby for a WV hillbilly
Yahoo.com attorneys please note, additions to this message
by Gene Heskett are:
Copyright 2004 by Maurice Eugene Heskett, all rights reserved.

2004-10-18 22:29:27

by Matt Domsch

[permalink] [raw]
Subject: dupes (was Re: using crypto_digest() on non-kmalloc'd memory failures)

On Mon, Oct 18, 2004 at 06:10:41PM -0400, Gene Heskett wrote:
> Off topic Matt, but why am I getting 4 to 6 copies of the messages you
> send? All identical time stamps, the whole works.

I'm only sending one far as I can tell...
Has it happened for more than one message, and only from me?

--
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions linux.dell.com & http://www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

2004-10-18 23:05:18

by James Morris

[permalink] [raw]
Subject: Re: using crypto_digest() on non-kmalloc'd memory failures

On Tue, 19 Oct 2004, Oleg Makarenko wrote:

> And one more question on crypto api. It looks like it is not very
> effective for a single byte "block" ciphers as arc4. The overhead is
> probably too big. Just look at the loop in cipher.c/crypt() and the code
> in arc4.c/arc4_crypt(). All this code is called for every single clear
> text byte. Right? Looks like an overkill for bsize == 1.
>
> Is there any better way to use crypto api for arc4 or similar ciphers?
> Cipher block size is not always a natural choice for the crypto_yield().
> Especially for fast ciphers (arc4) and small "block" sizes (arc4 again).

ARC4 is a bit strange because it's a stream cipher. I guess we could add
another encryption mode 'stream' which is optimized for one byte at a time
operation.


- James
--
James Morris
<[email protected]>


2004-10-19 05:41:39

by Oleg Makarenko

[permalink] [raw]
Subject: Re: using crypto_digest() on non-kmalloc'd memory failures

James Morris wrote:

On Tue, 19 Oct 2004, Oleg Makarenko wrote:


>>Is there any better way to use crypto api for arc4 or similar ciphers?
>>Cipher block size is not always a natural choice for the crypto_yield().
>>Especially for fast ciphers (arc4) and small "block" sizes (arc4 again).
>>
>>
>
>ARC4 is a bit strange because it's a stream cipher. I guess we could add
>another encryption mode 'stream' which is optimized for one byte at a time
>operation.
>
>
>- James
>
>
That would probably require one more parameter to
cin_encrypt/cia_decrypt, something like "nblocks" which currently is
always 1 and one more struct crypto_alg field that would help upper
level to decide how many blocks it can safely encrypt/decrypt() at a
time before crypto_yield(). This value depends on algorithm speed and
should be chosen to make overhead smaller. It could be > 1 even for
fast block ciphers and should be > 1 for stream ciphers.

=oleg