All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* kernel tainted while exporting shash context using af_alg interface
@ 2015-10-25  6:26 Harsh Jain
  2015-10-25 11:58 ` Stephan Mueller
  0 siblings, 1 reply; 19+ messages in thread
From: Harsh Jain @ 2015-10-25  6:26 UTC (permalink / raw
  To: linux-crypto

Hi,


When trying to calculate HMAC(SHA1) with openssl using af-alg engine
kernel crashes.Find below the command used and kernel. I have added
some debug print in logs.

Command used : ./openssl dgst -engine af_alg -sha1 -hmac "key" r.txt
kernel version : 3.17.8

Initial Investigation : In shash_desc structure tfm pointer contains
5a5a5a5a5a5a5a5a(invalid pointer), when it triess to reference export
function pointer it crashes. As per my understanding tfm object should
have pointer of memory bloack allocated in
"crypto_init_shash_ops_async" function

Please give some pointers to debug the issue.Any documentation to
understand the crypto-api code.

Kernel logs:


[ 3190.053499] Harsh : crypto_ahash_export
[ 3190.053499]  crypto_ahash_reqtfm
[ 3190.053500] req->base.tfm  ffff880193478ec8
[ 3190.053500] __crypto_ahash_cast=req->base.tfm  ffff880193478e80
[ 3190.053501] ahash_request_ctx{req} ffff8800362ebb98
[ 3190.053501] crypto_shash_alg entered
[ 3190.053502] crypto_shash_alg TFM  5a5a5a5a5a5a5a5a
[ 3190.053502] crypto_shash_alg TFM Base 5a5a5a5a5a5a5a62
[ 3190.053507] general protection fault: 0000 [#1] SMP
[ 3190.053509] Modules linked in: coretemp kvm_intel kvm crc32c_intel
iTCO_wdt iTCO_vendor_support ppdev parport_pc parport i2c_i801 lpc_ich
microcode i2c_core serio_raw mfd_core tpm_infineon pcspkr shpchp
ioatdma i7core_edac edac_core dca acpi_cpufreq uinput xfs libcrc32c
exportfs sd_mod sr_mod crc_t10dif crct10dif_common cdrom ata_generic
pata_acpi e1000e ptp ata_piix mptsas scsi_transport_sas mptscsih
mptbase dm_mirror dm_region_hash dm_log dm_mod ipv6 autofs4
[ 3190.053527] CPU: 1 PID: 3043 Comm: openssl Not tainted 3.17.8_harsh #39
[ 3190.053528] Hardware name: Supermicro X8ST3/X8ST3, BIOS 2.0a       11/28/2012
[ 3190.053529] task: ffff88019866e2d0 ti: ffff8800d71e4000 task.ti:
ffff8800d71e4000
[ 3190.053530] RIP: 0010:[<ffffffff81273bce>]  [<ffffffff81273bce>]
shash_async_export+0x5e/0x90
[ 3190.053533] RSP: 0018:ffff8800d71e7de8  EFLAGS: 00010282
[ 3190.053533] RAX: 000000000000002a RBX: 5a5a5a5a5a5a5a5a RCX: 0000000000000006
[ 3190.053534] RDX: 0000000000000000 RSI: 0000000000000246 RDI: ffff88019fc2c130
[ 3190.053535] RBP: ffff8800d71e7e00 R08: 0000000000000400 R09: ffffffff81db9f64
[ 3190.053536] R10: 0000000000000ced R11: 0000000000000cec R12: ffff8800362ebb98
[ 3190.053536] R13: ffff8800d71e7e10 R14: ffff8800d74bfcc0 R15: ffff8800362ebb48
[ 3190.053537] FS:  00007ffff7fe1740(0000) GS:ffff88019fc20000(0000)
knlGS:0000000000000000
[ 3190.053538] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 3190.053539] CR2: 0000000001c3baf8 CR3: 00000000daa8b000 CR4: 00000000000007e0
[ 3190.053540] Stack:
[ 3190.053540]  ffff8800362eb800 ffff880193478ec8 ffff8800d61b7800
ffff8800d71e7e98
[ 3190.053542]  ffffffff81295adc ffffffff815c3d53 0000000000000020
ffff8800d71e7e70
[ 3190.053543]  ffff8800d71e7e30 ffffffff811b5fab ffff880193478ec8
ffffffff81ad4cd0
[ 3190.053545] Call Trace:
[ 3190.053547]  [<ffffffff81295adc>] hash_accept+0x13c/0x250
[ 3190.053548]  [<ffffffff815c3d53>] ? printk+0x54/0x56
[ 3190.053549]  [<ffffffff811b5fab>] ? alloc_file+0x1b/0xc0
[ 3190.053550]  [<ffffffff81295a25>] ? hash_accept+0x85/0x250
[ 3190.053552]  [<ffffffff814e2a94>] SYSC_accept4+0xf4/0x200
[ 3190.053553]  [<ffffffff811b414c>] ? vfs_write+0x15c/0x1f0
[ 3190.053555]  [<ffffffff814e4010>] SyS_accept+0x10/0x20
[ 3190.053556]  [<ffffffff815cfe52>] system_call_fastpath+0x16/0x1b
[ 3190.053557] Code: 81 31 c0 e8 54 01 35 00 48 c7 c7 6e 61 82 81 31
c0 48 89 de e8 43 01 35 00 48 8d 73 08 48 c7 c7 e8 bd 82 81 31 c0 e8
31 01 35 00 <48> 8b 73 58 48 c7 c7 08 be 82 81 31 c0 e8 1f 01 35 00 48
8b 43
[ 3190.053575] RIP  [<ffffffff81273bce>] shash_async_export+0x5e/0x90
[ 3190.053577]  RSP <ffff8800d71e7de8>
[ 3190.053578] ---[ end trace d9701f2848d12eb5 ]---

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: kernel tainted while exporting shash context using af_alg interface
  2015-10-25  6:26 kernel tainted while exporting shash context using af_alg interface Harsh Jain
@ 2015-10-25 11:58 ` Stephan Mueller
  2015-10-26  6:19   ` Harsh Jain
  0 siblings, 1 reply; 19+ messages in thread
From: Stephan Mueller @ 2015-10-25 11:58 UTC (permalink / raw
  To: Harsh Jain; +Cc: linux-crypto

Am Sonntag, 25. Oktober 2015, 11:56:27 schrieb Harsh Jain:

Hi Harsh,

>Hi,
>
>
>When trying to calculate HMAC(SHA1) with openssl using af-alg engine
>kernel crashes.Find below the command used and kernel. I have added
>some debug print in logs.
>
>Command used : ./openssl dgst -engine af_alg -sha1 -hmac "key" r.txt
>kernel version : 3.17.8
>
>Initial Investigation : In shash_desc structure tfm pointer contains
>5a5a5a5a5a5a5a5a(invalid pointer), when it triess to reference export
>function pointer it crashes. As per my understanding tfm object should
>have pointer of memory bloack allocated in
>"crypto_init_shash_ops_async" function
>
>Please give some pointers to debug the issue.Any documentation to
>understand the crypto-api code.

May I ask you to send 2 things: the source code of the OpenSSL af_alg engine 
that you use (IIRC it is not included upstream, I want to be sure I used the 
right one).

Further, can you attach an strace of the aforementioned command?

Note, I am playing and abusing the AF_ALG interface for quite some time with 
[1] but I did not come across any issues like the one you describe here.

[1] http://www.chronox.de/libkcapi.html

Ciao
Stephan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: kernel tainted while exporting shash context using af_alg interface
  2015-10-25 11:58 ` Stephan Mueller
@ 2015-10-26  6:19   ` Harsh Jain
  2015-10-26  9:21     ` Harsh Jain
  0 siblings, 1 reply; 19+ messages in thread
From: Harsh Jain @ 2015-10-26  6:19 UTC (permalink / raw
  To: Stephan Mueller; +Cc: linux-crypto

[-- Attachment #1: Type: text/plain, Size: 1991 bytes --]

Hi Stephan,

I also tried test program in libkcapi and it works. libkcapi opens
socket of type "hmac(sha1)" .Openssl opens multiple "sha1" type socket
and uses the partial results to calculate hmac.

 "crypto_ahash_init()" function initialises the *tfm variable in
crypto_shash structure.It gets called when user calls write() system
call. To give a try I updated the hash_accept() function and re-run.
This time kernel didn't crashed but result calculated is wrong.

How accept() sys call decide weather to call alg_accept() or hash_accept()?

Find attached patch and strace.Right now af_alg code is not accessible
to me. I will share it tomorrow.



Thanks and Regards
Harsh jain

On Sun, Oct 25, 2015 at 5:28 PM, Stephan Mueller <smueller@chronox.de> wrote:
> Am Sonntag, 25. Oktober 2015, 11:56:27 schrieb Harsh Jain:
>
> Hi Harsh,
>
>>Hi,
>>
>>
>>When trying to calculate HMAC(SHA1) with openssl using af-alg engine
>>kernel crashes.Find below the command used and kernel. I have added
>>some debug print in logs.
>>
>>Command used : ./openssl dgst -engine af_alg -sha1 -hmac "key" r.txt
>>kernel version : 3.17.8
>>
>>Initial Investigation : In shash_desc structure tfm pointer contains
>>5a5a5a5a5a5a5a5a(invalid pointer), when it triess to reference export
>>function pointer it crashes. As per my understanding tfm object should
>>have pointer of memory bloack allocated in
>>"crypto_init_shash_ops_async" function
>>
>>Please give some pointers to debug the issue.Any documentation to
>>understand the crypto-api code.
>
> May I ask you to send 2 things: the source code of the OpenSSL af_alg engine
> that you use (IIRC it is not included upstream, I want to be sure I used the
> right one).
>
> Further, can you attach an strace of the aforementioned command?
>
> Note, I am playing and abusing the AF_ALG interface for quite some time with
> [1] but I did not come across any issues like the one you describe here.
>
> [1] http://www.chronox.de/libkcapi.html
>
> Ciao
> Stephan

[-- Attachment #2: algif_hash.patch --]
[-- Type: application/octet-stream, Size: 506 bytes --]

--- crypto/algif_hash.c	2015-10-25 13:27:29.220383694 -0400
+++ crypto/algif_hash_patch.c	2015-10-25 13:26:46.436933241 -0400
@@ -217,6 +217,11 @@ static int hash_accept(struct socket *so
 	dump_stack();	
 	printk(KERN_DEBUG "+++++++++++++++++++++++++++++\n");
 
+	err = crypto_ahash_init(&ctx->req);
+                if (err) {
+			printk(KERN_DEBUG "hash_accept: Error in crypto_ahash_init\n");        
+	                return err;
+	}
 	err = crypto_ahash_export(req, state);
 	if (err)
 		return err;

[-- Attachment #3: strace.log --]
[-- Type: application/octet-stream, Size: 9092 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: kernel tainted while exporting shash context using af_alg interface
  2015-10-26  6:19   ` Harsh Jain
@ 2015-10-26  9:21     ` Harsh Jain
  2015-10-28  0:09       ` Stephan Mueller
  0 siblings, 1 reply; 19+ messages in thread
From: Harsh Jain @ 2015-10-26  9:21 UTC (permalink / raw
  To: Stephan Mueller; +Cc: linux-crypto

[-- Attachment #1: Type: text/plain, Size: 2294 bytes --]

Hi Stephan,

I tried 1 more patch. This time result is correct. Find attached patch
file. Is there any side effect of this patch.


Regards
Harsh Jain

On Mon, Oct 26, 2015 at 11:49 AM, Harsh Jain <harshjain.prof@gmail.com> wrote:
> Hi Stephan,
>
> I also tried test program in libkcapi and it works. libkcapi opens
> socket of type "hmac(sha1)" .Openssl opens multiple "sha1" type socket
> and uses the partial results to calculate hmac.
>
>  "crypto_ahash_init()" function initialises the *tfm variable in
> crypto_shash structure.It gets called when user calls write() system
> call. To give a try I updated the hash_accept() function and re-run.
> This time kernel didn't crashed but result calculated is wrong.
>
> How accept() sys call decide weather to call alg_accept() or hash_accept()?
>
> Find attached patch and strace.Right now af_alg code is not accessible
> to me. I will share it tomorrow.
>
>
>
> Thanks and Regards
> Harsh jain
>
> On Sun, Oct 25, 2015 at 5:28 PM, Stephan Mueller <smueller@chronox.de> wrote:
>> Am Sonntag, 25. Oktober 2015, 11:56:27 schrieb Harsh Jain:
>>
>> Hi Harsh,
>>
>>>Hi,
>>>
>>>
>>>When trying to calculate HMAC(SHA1) with openssl using af-alg engine
>>>kernel crashes.Find below the command used and kernel. I have added
>>>some debug print in logs.
>>>
>>>Command used : ./openssl dgst -engine af_alg -sha1 -hmac "key" r.txt
>>>kernel version : 3.17.8
>>>
>>>Initial Investigation : In shash_desc structure tfm pointer contains
>>>5a5a5a5a5a5a5a5a(invalid pointer), when it triess to reference export
>>>function pointer it crashes. As per my understanding tfm object should
>>>have pointer of memory bloack allocated in
>>>"crypto_init_shash_ops_async" function
>>>
>>>Please give some pointers to debug the issue.Any documentation to
>>>understand the crypto-api code.
>>
>> May I ask you to send 2 things: the source code of the OpenSSL af_alg engine
>> that you use (IIRC it is not included upstream, I want to be sure I used the
>> right one).
>>
>> Further, can you attach an strace of the aforementioned command?
>>
>> Note, I am playing and abusing the AF_ALG interface for quite some time with
>> [1] but I did not come across any issues like the one you describe here.
>>
>> [1] http://www.chronox.de/libkcapi.html
>>
>> Ciao
>> Stephan

[-- Attachment #2: algif_hash_2.patch --]
[-- Type: application/octet-stream, Size: 517 bytes --]

--- algif_hash_orig.c	2015-10-25 13:27:29.220383694 -0400
+++ algif_hash.c	2015-10-26 04:38:38.422923347 -0400
@@ -217,6 +217,13 @@ static int hash_accept(struct socket *so
 	dump_stack();	
 	printk(KERN_DEBUG "+++++++++++++++++++++++++++++\n");
 
+	if (!ctx->more) {
+		err = crypto_ahash_init(&ctx->req);
+                if (err) {
+			printk(KERN_DEBUG "hash_accept: Error in crypto_ahash_init\n");        
+	                return err;
+		}
+	}
 	err = crypto_ahash_export(req, state);
 	if (err)
 		return err;

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: kernel tainted while exporting shash context using af_alg interface
  2015-10-26  9:21     ` Harsh Jain
@ 2015-10-28  0:09       ` Stephan Mueller
  2015-10-28  0:55         ` Stephan Mueller
  0 siblings, 1 reply; 19+ messages in thread
From: Stephan Mueller @ 2015-10-28  0:09 UTC (permalink / raw
  To: Harsh Jain; +Cc: linux-crypto

Am Montag, 26. Oktober 2015, 14:51:01 schrieb Harsh Jain:

Hi Harsh,

> Hi Stephan,
> 
> I tried 1 more patch. This time result is correct. Find attached patch
> file. Is there any side effect of this patch.

The strace is enlightening.

The user space code does an accept on an already accepted FD

It seems your user space does something like:

socket()
fd = bind()
fd1 = accept(fd)
fd2 = accept(fd1)
fd3 = accept(fd2)
...

That is an error in the user space code. The correct way would be like the 
code in [1] with all the lines until the line 553 (the code afterwards is for 
vmsplice).

So, the code goes like that:

tfmfd = socket()
bind(tfmfd)
opfd = accept(tfmfd);

>From now on, you use opfd for all sendmsg/recvmsg operations.


However, any error in user space should not crash the kernel. So, a fix should 
be done. But I think your code is not correct as it solidifies a broken user 
space code.

I would rather think the following patch should be added to prevent the oops. 
At least for me, multiple accepts does not crash the kernel. Can you please 
test whether this patch ensures you kernel stays sane?

diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index 1396ad0..785df23 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -183,6 +183,9 @@ static int hash_accept(struct socket *sock, struct socket 
*newsock, int flags)
 	struct hash_ctx *ctx2;
 	int err;
 
+	if (!ctx->more)
+		return -EINVAL;
+
 	err = crypto_ahash_export(req, state);
 	if (err)
 		return err;


[1] https://github.com/smuellerDD/libkcapi/blob/master/lib/kcapi-kernel-if.c#L534


-- 
Ciao
Stephan

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: kernel tainted while exporting shash context using af_alg interface
  2015-10-28  0:09       ` Stephan Mueller
@ 2015-10-28  0:55         ` Stephan Mueller
  2015-10-28 10:54           ` Harsh Jain
  0 siblings, 1 reply; 19+ messages in thread
From: Stephan Mueller @ 2015-10-28  0:55 UTC (permalink / raw
  To: Harsh Jain, herbert; +Cc: linux-crypto

Am Mittwoch, 28. Oktober 2015, 01:09:58 schrieb Stephan Mueller:

Hi Harsh,

> 
> 
> However, any error in user space should not crash the kernel. So, a fix
> should be done. But I think your code is not correct as it solidifies a
> broken user space code.

After thinking a bit again, I think your approach is correct after all. I was 
able to reproduce the crash by simply adding more accept calls to my test 
code. And I can confirm that your patch works, for hashes.

*BUT* it does NOT work for HMAC as the key is set on the TFM and the 
subsequent accepts do not transport the key. Albeit your code prevents the 
kernel from crashing, the HMAC calculation will be done with an empty key as 
the setkey operation does not reach the TFM handle in the subordinate accept() 
call.

So, I would think that the second accept is simply broken, for HMAC at least.

Herbert, what is the purpose of that subordinate accept that is implemented 
with hash_accept? As this is broken for HMACs, should it be removed entirely?

-- 
Ciao
Stephan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: kernel tainted while exporting shash context using af_alg interface
  2015-10-28  0:55         ` Stephan Mueller
@ 2015-10-28 10:54           ` Harsh Jain
  2015-10-28 11:23             ` Stephan Mueller
  0 siblings, 1 reply; 19+ messages in thread
From: Harsh Jain @ 2015-10-28 10:54 UTC (permalink / raw
  To: Stephan Mueller; +Cc: herbert, linux-crypto

Hi Stephan,

I tried your patch on my machine. Kernel is not crashing. The openssl
break with this. Can you share HMAC program which you are suspecting
it will not work or do you already have some test written in
libkcapi/test.sh which will fail.


Regards
Harsh Jain

On Wed, Oct 28, 2015 at 6:25 AM, Stephan Mueller <smueller@chronox.de> wrote:
> Am Mittwoch, 28. Oktober 2015, 01:09:58 schrieb Stephan Mueller:
>
> Hi Harsh,
>
>>
>>
>> However, any error in user space should not crash the kernel. So, a fix
>> should be done. But I think your code is not correct as it solidifies a
>> broken user space code.
>
> After thinking a bit again, I think your approach is correct after all. I was
> able to reproduce the crash by simply adding more accept calls to my test
> code. And I can confirm that your patch works, for hashes.
>
> *BUT* it does NOT work for HMAC as the key is set on the TFM and the
> subsequent accepts do not transport the key. Albeit your code prevents the
> kernel from crashing, the HMAC calculation will be done with an empty key as
> the setkey operation does not reach the TFM handle in the subordinate accept()
> call.
>
> So, I would think that the second accept is simply broken, for HMAC at least.
>
> Herbert, what is the purpose of that subordinate accept that is implemented
> with hash_accept? As this is broken for HMACs, should it be removed entirely?
>
> --
> Ciao
> Stephan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: kernel tainted while exporting shash context using af_alg interface
  2015-10-28 10:54           ` Harsh Jain
@ 2015-10-28 11:23             ` Stephan Mueller
  2015-10-30  8:32               ` Harsh Jain
  0 siblings, 1 reply; 19+ messages in thread
From: Stephan Mueller @ 2015-10-28 11:23 UTC (permalink / raw
  To: Harsh Jain; +Cc: herbert, linux-crypto

Am Mittwoch, 28. Oktober 2015, 16:24:34 schrieb Harsh Jain:

Hi Harsh,

>Hi Stephan,
>
>I tried your patch on my machine. Kernel is not crashing. The openssl
>break with this. Can you share HMAC program which you are suspecting
>it will not work or do you already have some test written in
>libkcapi/test.sh which will fail.

See comments above test/kcapi-main.c:cavs_hash

 * HMAC command line invocation:
 * $ ./kcapi -x 3 -c "hmac(sha1)" -k 6e77ebd479da794707bc6cde3694f552ea892dab 
-p  
31b62a797adbff6b8a358d2b5206e01fee079de8cdfc4695138bba163b4efbf30127343e7fd4fbc696c3d38d8f27f57c024b5056f726ceeb4c31d98e57751ec8cbe8904ee0f9b031ae6a0c55da5e062475b3d7832191d4057643ef5fa446801d59a04693e573a8159cd2416b7bd39c7f0fe63c599365e04d596c05736beaab58
 * 7f204ea665666f5bd2b370e546d1b408005e4d85

To do that, apply your patch and then

1. open lib/kcapi-kernel-if.c and change line 567 from

handle->opfd = accept(handle->tfmfd, NULL, 0);


to

handle->opfd = accept(handle->tfmfd, NULL, 0);
handle->opfd = accept(handle->opfd, NULL, 0);
handle->opfd = accept(handle->opfd, NULL, 0);
handle->opfd = accept(handle->opfd, NULL, 0);
handle->opfd = accept(handle->opfd, NULL, 0);

You will see that the hash commands will pass, the HMAC fails

Without your patch, the kernel crashes (same as with your OpenSSL code).

The reason is that setkey is applied on the TFM that is not conveyed to the 
subsequent TFMs generated with new accepts.
>
>
>Regards
>Harsh Jain
>
>On Wed, Oct 28, 2015 at 6:25 AM, Stephan Mueller <smueller@chronox.de> wrote:
>> Am Mittwoch, 28. Oktober 2015, 01:09:58 schrieb Stephan Mueller:
>> 
>> Hi Harsh,
>> 
>>> However, any error in user space should not crash the kernel. So, a fix
>>> should be done. But I think your code is not correct as it solidifies a
>>> broken user space code.
>> 
>> After thinking a bit again, I think your approach is correct after all. I
>> was able to reproduce the crash by simply adding more accept calls to my
>> test code. And I can confirm that your patch works, for hashes.
>> 
>> *BUT* it does NOT work for HMAC as the key is set on the TFM and the
>> subsequent accepts do not transport the key. Albeit your code prevents the
>> kernel from crashing, the HMAC calculation will be done with an empty key
>> as
>> the setkey operation does not reach the TFM handle in the subordinate
>> accept() call.
>> 
>> So, I would think that the second accept is simply broken, for HMAC at
>> least.
>> 
>> Herbert, what is the purpose of that subordinate accept that is implemented
>> with hash_accept? As this is broken for HMACs, should it be removed
>> entirely?
>> 
>> --
>> Ciao
>> Stephan
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html


Ciao
Stephan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: kernel tainted while exporting shash context using af_alg interface
  2015-10-28 11:23             ` Stephan Mueller
@ 2015-10-30  8:32               ` Harsh Jain
  2015-10-30 11:10                 ` Stephan Mueller
  0 siblings, 1 reply; 19+ messages in thread
From: Harsh Jain @ 2015-10-30  8:32 UTC (permalink / raw
  To: Stephan Mueller; +Cc: Herbert Xu, linux-crypto

Hi Stephan,

If we add sendmsg() in between 2 accept calls then the setkey problem
will happen?

handle->opfd = accept(handle->tfmfd, NULL, 0);
sendmsg()
handle->opfd = accept(handle->opfd, NULL, 0);
sendmsg()
handle->opfd = accept(handle->opfd, NULL, 0);

If yes, Then may be it is expected behavior and user is supposed to
set the key explicitly with some other system call.Why I am saying
this is. I remember somewhere in kernel code I read some comment
related to setkey operations.

In that case my patch should work. 1 doubt I have related to patch is
do I need to set "ctx->more" =1 after initialisation.

Correct me If I am wrong.


Thanks for your support.


regards
Harsh Jain

On Wed, Oct 28, 2015 at 4:53 PM, Stephan Mueller <smueller@chronox.de> wrote:
> Am Mittwoch, 28. Oktober 2015, 16:24:34 schrieb Harsh Jain:
>
> Hi Harsh,
>
>>Hi Stephan,
>>
>>I tried your patch on my machine. Kernel is not crashing. The openssl
>>break with this. Can you share HMAC program which you are suspecting
>>it will not work or do you already have some test written in
>>libkcapi/test.sh which will fail.
>
> See comments above test/kcapi-main.c:cavs_hash
>
>  * HMAC command line invocation:
>  * $ ./kcapi -x 3 -c "hmac(sha1)" -k 6e77ebd479da794707bc6cde3694f552ea892dab
> -p
> 31b62a797adbff6b8a358d2b5206e01fee079de8cdfc4695138bba163b4efbf30127343e7fd4fbc696c3d38d8f27f57c024b5056f726ceeb4c31d98e57751ec8cbe8904ee0f9b031ae6a0c55da5e062475b3d7832191d4057643ef5fa446801d59a04693e573a8159cd2416b7bd39c7f0fe63c599365e04d596c05736beaab58
>  * 7f204ea665666f5bd2b370e546d1b408005e4d85
>
> To do that, apply your patch and then
>
> 1. open lib/kcapi-kernel-if.c and change line 567 from
>
> handle->opfd = accept(handle->tfmfd, NULL, 0);
>
>
> to
>
> handle->opfd = accept(handle->tfmfd, NULL, 0);
> handle->opfd = accept(handle->opfd, NULL, 0);
> handle->opfd = accept(handle->opfd, NULL, 0);
> handle->opfd = accept(handle->opfd, NULL, 0);
> handle->opfd = accept(handle->opfd, NULL, 0);
>
> You will see that the hash commands will pass, the HMAC fails
>
> Without your patch, the kernel crashes (same as with your OpenSSL code).
>
> The reason is that setkey is applied on the TFM that is not conveyed to the
> subsequent TFMs generated with new accepts.
>>
>>
>>Regards
>>Harsh Jain
>>
>>On Wed, Oct 28, 2015 at 6:25 AM, Stephan Mueller <smueller@chronox.de> wrote:
>>> Am Mittwoch, 28. Oktober 2015, 01:09:58 schrieb Stephan Mueller:
>>>
>>> Hi Harsh,
>>>
>>>> However, any error in user space should not crash the kernel. So, a fix
>>>> should be done. But I think your code is not correct as it solidifies a
>>>> broken user space code.
>>>
>>> After thinking a bit again, I think your approach is correct after all. I
>>> was able to reproduce the crash by simply adding more accept calls to my
>>> test code. And I can confirm that your patch works, for hashes.
>>>
>>> *BUT* it does NOT work for HMAC as the key is set on the TFM and the
>>> subsequent accepts do not transport the key. Albeit your code prevents the
>>> kernel from crashing, the HMAC calculation will be done with an empty key
>>> as
>>> the setkey operation does not reach the TFM handle in the subordinate
>>> accept() call.
>>>
>>> So, I would think that the second accept is simply broken, for HMAC at
>>> least.
>>>
>>> Herbert, what is the purpose of that subordinate accept that is implemented
>>> with hash_accept? As this is broken for HMACs, should it be removed
>>> entirely?
>>>
>>> --
>>> Ciao
>>> Stephan
>>
>>--
>>To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
>>the body of a message to majordomo@vger.kernel.org
>>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
> Ciao
> Stephan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: kernel tainted while exporting shash context using af_alg interface
  2015-10-30  8:32               ` Harsh Jain
@ 2015-10-30 11:10                 ` Stephan Mueller
  2015-10-30 12:16                   ` crypto: algif_hash - Only export and import on sockets with data Herbert Xu
  2015-11-02  5:48                   ` kernel tainted while exporting shash context using af_alg interface Harsh Jain
  0 siblings, 2 replies; 19+ messages in thread
From: Stephan Mueller @ 2015-10-30 11:10 UTC (permalink / raw
  To: Harsh Jain; +Cc: Herbert Xu, linux-crypto

Am Freitag, 30. Oktober 2015, 14:02:27 schrieb Harsh Jain:

Hi Harsh,

>Hi Stephan,
>
>If we add sendmsg() in between 2 accept calls then the setkey problem
>will happen?
>
>handle->opfd = accept(handle->tfmfd, NULL, 0);
>sendmsg()
>handle->opfd = accept(handle->opfd, NULL, 0);
>sendmsg()
>handle->opfd = accept(handle->opfd, NULL, 0);

Without testing, I would very much expect that, because the setkey does not 
apply to the subordinate tfm.
>
>If yes, Then may be it is expected behavior and user is supposed to
>set the key explicitly with some other system call.Why I am saying
>this is. I remember somewhere in kernel code I read some comment
>related to setkey operations.

I would like to wait for Herbert to chime in here on how he thinks this would 
work.
>
>In that case my patch should work. 1 doubt I have related to patch is
>do I need to set "ctx->more" =1 after initialisation.
>
>Correct me If I am wrong.
>
>
>Thanks for your support.
>
>
>regards
>Harsh Jain
>
>On Wed, Oct 28, 2015 at 4:53 PM, Stephan Mueller <smueller@chronox.de> wrote:
>> Am Mittwoch, 28. Oktober 2015, 16:24:34 schrieb Harsh Jain:
>> 
>> Hi Harsh,
>> 
>>>Hi Stephan,
>>>
>>>I tried your patch on my machine. Kernel is not crashing. The openssl
>>>break with this. Can you share HMAC program which you are suspecting
>>>it will not work or do you already have some test written in
>>>libkcapi/test.sh which will fail.
>>>
>> See comments above test/kcapi-main.c:cavs_hash
>> 
>>  * HMAC command line invocation:
>>  * $ ./kcapi -x 3 -c "hmac(sha1)" -k
>>  6e77ebd479da794707bc6cde3694f552ea892dab
>> 
>> -p
>> 31b62a797adbff6b8a358d2b5206e01fee079de8cdfc4695138bba163b4efbf30127343e7fd
>> 4fbc696c3d38d8f27f57c024b5056f726ceeb4c31d98e57751ec8cbe8904ee0f9b031ae6a0c
>> 55da5e062475b3d7832191d4057643ef5fa446801d59a04693e573a8159cd2416b7bd39c7f0
>> fe63c599365e04d596c05736beaab58> 
>>  * 7f204ea665666f5bd2b370e546d1b408005e4d85
>> 
>> To do that, apply your patch and then
>> 
>> 1. open lib/kcapi-kernel-if.c and change line 567 from
>> 
>> handle->opfd = accept(handle->tfmfd, NULL, 0);
>> 
>> 
>> to
>> 
>> handle->opfd = accept(handle->tfmfd, NULL, 0);
>> handle->opfd = accept(handle->opfd, NULL, 0);
>> handle->opfd = accept(handle->opfd, NULL, 0);
>> handle->opfd = accept(handle->opfd, NULL, 0);
>> handle->opfd = accept(handle->opfd, NULL, 0);
>> 
>> You will see that the hash commands will pass, the HMAC fails
>> 
>> Without your patch, the kernel crashes (same as with your OpenSSL code).
>> 
>> The reason is that setkey is applied on the TFM that is not conveyed to the
>> subsequent TFMs generated with new accepts.
>> 
>>>Regards
>>>Harsh Jain
>>>
>>>On Wed, Oct 28, 2015 at 6:25 AM, Stephan Mueller <smueller@chronox.de> 
wrote:
>>>> Am Mittwoch, 28. Oktober 2015, 01:09:58 schrieb Stephan Mueller:
>>>> 
>>>> Hi Harsh,
>>>> 
>>>>> However, any error in user space should not crash the kernel. So, a fix
>>>>> should be done. But I think your code is not correct as it solidifies a
>>>>> broken user space code.
>>>> 
>>>> After thinking a bit again, I think your approach is correct after all. I
>>>> was able to reproduce the crash by simply adding more accept calls to my
>>>> test code. And I can confirm that your patch works, for hashes.
>>>> 
>>>> *BUT* it does NOT work for HMAC as the key is set on the TFM and the
>>>> subsequent accepts do not transport the key. Albeit your code prevents
>>>> the
>>>> kernel from crashing, the HMAC calculation will be done with an empty key
>>>> as
>>>> the setkey operation does not reach the TFM handle in the subordinate
>>>> accept() call.
>>>> 
>>>> So, I would think that the second accept is simply broken, for HMAC at
>>>> least.
>>>> 
>>>> Herbert, what is the purpose of that subordinate accept that is
>>>> implemented
>>>> with hash_accept? As this is broken for HMACs, should it be removed
>>>> entirely?
>>>> 
>>>> --
>>>> Ciao
>>>> Stephan
>>>
>>>--
>>>To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
>>>the body of a message to majordomo@vger.kernel.org
>>>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>> Ciao
>> Stephan


Ciao
Stephan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* crypto: algif_hash - Only export and import on sockets with data
  2015-10-30 11:10                 ` Stephan Mueller
@ 2015-10-30 12:16                   ` Herbert Xu
  2015-10-30 23:45                     ` Stephan Mueller
  2015-11-02  5:48                   ` kernel tainted while exporting shash context using af_alg interface Harsh Jain
  1 sibling, 1 reply; 19+ messages in thread
From: Herbert Xu @ 2015-10-30 12:16 UTC (permalink / raw
  To: Stephan Mueller; +Cc: Harsh Jain, linux-crypto

On Fri, Oct 30, 2015 at 12:10:51PM +0100, Stephan Mueller wrote:
> Am Freitag, 30. Oktober 2015, 14:02:27 schrieb Harsh Jain:
> 
> Hi Harsh,
> 
> >Hi Stephan,
> >
> >If we add sendmsg() in between 2 accept calls then the setkey problem
> >will happen?
> >
> >handle->opfd = accept(handle->tfmfd, NULL, 0);
> >sendmsg()
> >handle->opfd = accept(handle->opfd, NULL, 0);
> >sendmsg()
> >handle->opfd = accept(handle->opfd, NULL, 0);
> 
> Without testing, I would very much expect that, because the setkey does not 
> apply to the subordinate tfm.

setkey should be needed as the subsequent accept will all be based
on the same parent fd, meaning that they will all use a single tfm.

Please try the following patch.

---8<---
The hash_accept call fails to work on sockets that have not received
any data.  For some algorithm implementations it may cause crashes.

This patch fixes this by ensuring that we only export and import on
sockets that have received data.

Cc: stable@vger.kernel.org
Reported-by: Harsh Jain <harshjain.prof@gmail.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index 1396ad0..bbda6b4 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -181,11 +181,14 @@ static int hash_accept(struct socket *sock, struct socket *newsock, int flags)
 	struct sock *sk2;
 	struct alg_sock *ask2;
 	struct hash_ctx *ctx2;
+	bool more = ctx->more;
 	int err;
 
-	err = crypto_ahash_export(req, state);
-	if (err)
-		return err;
+	if (more) {
+		err = crypto_ahash_export(req, state);
+		if (err)
+			return err;
+	}
 
 	err = af_alg_accept(ask->parent, newsock);
 	if (err)
@@ -194,12 +197,14 @@ static int hash_accept(struct socket *sock, struct socket *newsock, int flags)
 	sk2 = newsock->sk;
 	ask2 = alg_sk(sk2);
 	ctx2 = ask2->private;
-	ctx2->more = 1;
+	ctx2->more = more;
 
-	err = crypto_ahash_import(&ctx2->req, state);
-	if (err) {
-		sock_orphan(sk2);
-		sock_put(sk2);
+	if (more) {
+		err = crypto_ahash_import(&ctx2->req, state);
+		if (err) {
+			sock_orphan(sk2);
+			sock_put(sk2);
+		}
 	}
 
 	return err;
-- 
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] 19+ messages in thread

* Re: crypto: algif_hash - Only export and import on sockets with data
  2015-10-30 12:16                   ` crypto: algif_hash - Only export and import on sockets with data Herbert Xu
@ 2015-10-30 23:45                     ` Stephan Mueller
  2015-11-01  9:11                       ` [PATCH v2] " Herbert Xu
  0 siblings, 1 reply; 19+ messages in thread
From: Stephan Mueller @ 2015-10-30 23:45 UTC (permalink / raw
  To: Herbert Xu; +Cc: Harsh Jain, linux-crypto

Am Freitag, 30. Oktober 2015, 20:16:51 schrieb Herbert Xu:

Hi Herbert,

> 
> setkey should be needed as the subsequent accept will all be based
> on the same parent fd, meaning that they will all use a single tfm.
> 
> Please try the following patch.

Testing complete: patch solves the oops and allows to successfully perform 
HMAC even when having subsequent accepts and operating on those subsequent 
accepts.

-- 
Ciao
Stephan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2] crypto: algif_hash - Only export and import on sockets with data
  2015-10-30 23:45                     ` Stephan Mueller
@ 2015-11-01  9:11                       ` Herbert Xu
  2015-11-01 11:06                         ` Stephan Mueller
  2015-11-01 11:07                         ` Stephan Mueller
  0 siblings, 2 replies; 19+ messages in thread
From: Herbert Xu @ 2015-11-01  9:11 UTC (permalink / raw
  To: Stephan Mueller; +Cc: Harsh Jain, linux-crypto

On Sat, Oct 31, 2015 at 12:45:47AM +0100, Stephan Mueller wrote:
> 
> Testing complete: patch solves the oops and allows to successfully perform 
> HMAC even when having subsequent accepts and operating on those subsequent 
> accepts.

Thanks Stephan!

Unfortunately my patch is incomplete as some other thread could
change ctx->more while we're in the middle of the accept call.

So here is an updated version.

---8<---
The hash_accept call fails to work on sockets that have not received
any data.  For some algorithm implementations it may cause crashes.

This patch fixes this by ensuring that we only export and import on
sockets that have received data.

Cc: stable@vger.kernel.org
Reported-by: Harsh Jain <harshjain.prof@gmail.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index 1396ad0..b4c24fe 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -181,9 +181,14 @@ static int hash_accept(struct socket *sock, struct socket *newsock, int flags)
 	struct sock *sk2;
 	struct alg_sock *ask2;
 	struct hash_ctx *ctx2;
+	bool more;
 	int err;
 
-	err = crypto_ahash_export(req, state);
+	lock_sock(sk);
+	more = ctx->more;
+	err = more ? crypto_ahash_export(req, state) : 0;
+	release_sock(sk);
+
 	if (err)
 		return err;
 
@@ -194,7 +199,10 @@ static int hash_accept(struct socket *sock, struct socket *newsock, int flags)
 	sk2 = newsock->sk;
 	ask2 = alg_sk(sk2);
 	ctx2 = ask2->private;
-	ctx2->more = 1;
+	ctx2->more = more;
+
+	if (!more)
+		return err;
 
 	err = crypto_ahash_import(&ctx2->req, state);
 	if (err) {
-- 
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] 19+ messages in thread

* Re: [PATCH v2] crypto: algif_hash - Only export and import on sockets with data
  2015-11-01  9:11                       ` [PATCH v2] " Herbert Xu
@ 2015-11-01 11:06                         ` Stephan Mueller
  2015-11-01 11:07                         ` Stephan Mueller
  1 sibling, 0 replies; 19+ messages in thread
From: Stephan Mueller @ 2015-11-01 11:06 UTC (permalink / raw
  To: Herbert Xu; +Cc: Harsh Jain, linux-crypto

Am Sonntag, 1. November 2015, 17:11:19 schrieb Herbert Xu:

Hi Herbert,

> On Sat, Oct 31, 2015 at 12:45:47AM +0100, Stephan Mueller wrote:
> > Testing complete: patch solves the oops and allows to successfully perform
> > HMAC even when having subsequent accepts and operating on those subsequent
> > accepts.
> 
> Thanks Stephan!
> 
> Unfortunately my patch is incomplete as some other thread could
> change ctx->more while we're in the middle of the accept call.

Tested-by: Stephan Mueller <smueller@chronox.de>

-- 
Ciao
Stephan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] crypto: algif_hash - Only export and import on sockets with data
  2015-11-01  9:11                       ` [PATCH v2] " Herbert Xu
  2015-11-01 11:06                         ` Stephan Mueller
@ 2015-11-01 11:07                         ` Stephan Mueller
  2015-11-02  3:12                           ` Herbert Xu
  1 sibling, 1 reply; 19+ messages in thread
From: Stephan Mueller @ 2015-11-01 11:07 UTC (permalink / raw
  To: Herbert Xu; +Cc: Harsh Jain, linux-crypto

Am Sonntag, 1. November 2015, 17:11:19 schrieb Herbert Xu:

Hi Herbert,

> On Sat, Oct 31, 2015 at 12:45:47AM +0100, Stephan Mueller wrote:
> > Testing complete: patch solves the oops and allows to successfully perform
> > HMAC even when having subsequent accepts and operating on those subsequent
> > accepts.
> 
> Thanks Stephan!
> 
> Unfortunately my patch is incomplete as some other thread could
> change ctx->more while we're in the middle of the accept call.
> 
> So here is an updated version.

Shouldn't that patch to into stable (and hopefully into 4.3) as well?

-- 
Ciao
Stephan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] crypto: algif_hash - Only export and import on sockets with data
  2015-11-01 11:07                         ` Stephan Mueller
@ 2015-11-02  3:12                           ` Herbert Xu
  2015-11-05  7:42                             ` Harsh Jain
  0 siblings, 1 reply; 19+ messages in thread
From: Herbert Xu @ 2015-11-02  3:12 UTC (permalink / raw
  To: Stephan Mueller; +Cc: Harsh Jain, linux-crypto

On Sun, Nov 01, 2015 at 12:07:12PM +0100, Stephan Mueller wrote:
> Am Sonntag, 1. November 2015, 17:11:19 schrieb Herbert Xu:
> 
> Hi Herbert,
> 
> > On Sat, Oct 31, 2015 at 12:45:47AM +0100, Stephan Mueller wrote:
> > > Testing complete: patch solves the oops and allows to successfully perform
> > > HMAC even when having subsequent accepts and operating on those subsequent
> > > accepts.
> > 
> > Thanks Stephan!
> > 
> > Unfortunately my patch is incomplete as some other thread could
> > change ctx->more while we're in the middle of the accept call.
> > 
> > So here is an updated version.
> 
> Shouldn't that patch to into stable (and hopefully into 4.3) as well?

Yes it is going into stable.

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] 19+ messages in thread

* Re: kernel tainted while exporting shash context using af_alg interface
  2015-10-30 11:10                 ` Stephan Mueller
  2015-10-30 12:16                   ` crypto: algif_hash - Only export and import on sockets with data Herbert Xu
@ 2015-11-02  5:48                   ` Harsh Jain
  1 sibling, 0 replies; 19+ messages in thread
From: Harsh Jain @ 2015-11-02  5:48 UTC (permalink / raw
  To: Stephan Mueller; +Cc: Herbert Xu, linux-crypto

Hi,

I tried patch on my setup and its working fine.
Thanks Stephan, Herbert for your support.

Regards
Harsh Jain

On Fri, Oct 30, 2015 at 4:40 PM, Stephan Mueller <smueller@chronox.de> wrote:
> Am Freitag, 30. Oktober 2015, 14:02:27 schrieb Harsh Jain:
>
> Hi Harsh,
>
>>Hi Stephan,
>>
>>If we add sendmsg() in between 2 accept calls then the setkey problem
>>will happen?
>>
>>handle->opfd = accept(handle->tfmfd, NULL, 0);
>>sendmsg()
>>handle->opfd = accept(handle->opfd, NULL, 0);
>>sendmsg()
>>handle->opfd = accept(handle->opfd, NULL, 0);
>
> Without testing, I would very much expect that, because the setkey does not
> apply to the subordinate tfm.
>>
>>If yes, Then may be it is expected behavior and user is supposed to
>>set the key explicitly with some other system call.Why I am saying
>>this is. I remember somewhere in kernel code I read some comment
>>related to setkey operations.
>
> I would like to wait for Herbert to chime in here on how he thinks this would
> work.
>>
>>In that case my patch should work. 1 doubt I have related to patch is
>>do I need to set "ctx->more" =1 after initialisation.
>>
>>Correct me If I am wrong.
>>
>>
>>Thanks for your support.
>>
>>
>>regards
>>Harsh Jain
>>
>>On Wed, Oct 28, 2015 at 4:53 PM, Stephan Mueller <smueller@chronox.de> wrote:
>>> Am Mittwoch, 28. Oktober 2015, 16:24:34 schrieb Harsh Jain:
>>>
>>> Hi Harsh,
>>>
>>>>Hi Stephan,
>>>>
>>>>I tried your patch on my machine. Kernel is not crashing. The openssl
>>>>break with this. Can you share HMAC program which you are suspecting
>>>>it will not work or do you already have some test written in
>>>>libkcapi/test.sh which will fail.
>>>>
>>> See comments above test/kcapi-main.c:cavs_hash
>>>
>>>  * HMAC command line invocation:
>>>  * $ ./kcapi -x 3 -c "hmac(sha1)" -k
>>>  6e77ebd479da794707bc6cde3694f552ea892dab
>>>
>>> -p
>>> 31b62a797adbff6b8a358d2b5206e01fee079de8cdfc4695138bba163b4efbf30127343e7fd
>>> 4fbc696c3d38d8f27f57c024b5056f726ceeb4c31d98e57751ec8cbe8904ee0f9b031ae6a0c
>>> 55da5e062475b3d7832191d4057643ef5fa446801d59a04693e573a8159cd2416b7bd39c7f0
>>> fe63c599365e04d596c05736beaab58>
>>>  * 7f204ea665666f5bd2b370e546d1b408005e4d85
>>>
>>> To do that, apply your patch and then
>>>
>>> 1. open lib/kcapi-kernel-if.c and change line 567 from
>>>
>>> handle->opfd = accept(handle->tfmfd, NULL, 0);
>>>
>>>
>>> to
>>>
>>> handle->opfd = accept(handle->tfmfd, NULL, 0);
>>> handle->opfd = accept(handle->opfd, NULL, 0);
>>> handle->opfd = accept(handle->opfd, NULL, 0);
>>> handle->opfd = accept(handle->opfd, NULL, 0);
>>> handle->opfd = accept(handle->opfd, NULL, 0);
>>>
>>> You will see that the hash commands will pass, the HMAC fails
>>>
>>> Without your patch, the kernel crashes (same as with your OpenSSL code).
>>>
>>> The reason is that setkey is applied on the TFM that is not conveyed to the
>>> subsequent TFMs generated with new accepts.
>>>
>>>>Regards
>>>>Harsh Jain
>>>>
>>>>On Wed, Oct 28, 2015 at 6:25 AM, Stephan Mueller <smueller@chronox.de>
> wrote:
>>>>> Am Mittwoch, 28. Oktober 2015, 01:09:58 schrieb Stephan Mueller:
>>>>>
>>>>> Hi Harsh,
>>>>>
>>>>>> However, any error in user space should not crash the kernel. So, a fix
>>>>>> should be done. But I think your code is not correct as it solidifies a
>>>>>> broken user space code.
>>>>>
>>>>> After thinking a bit again, I think your approach is correct after all. I
>>>>> was able to reproduce the crash by simply adding more accept calls to my
>>>>> test code. And I can confirm that your patch works, for hashes.
>>>>>
>>>>> *BUT* it does NOT work for HMAC as the key is set on the TFM and the
>>>>> subsequent accepts do not transport the key. Albeit your code prevents
>>>>> the
>>>>> kernel from crashing, the HMAC calculation will be done with an empty key
>>>>> as
>>>>> the setkey operation does not reach the TFM handle in the subordinate
>>>>> accept() call.
>>>>>
>>>>> So, I would think that the second accept is simply broken, for HMAC at
>>>>> least.
>>>>>
>>>>> Herbert, what is the purpose of that subordinate accept that is
>>>>> implemented
>>>>> with hash_accept? As this is broken for HMACs, should it be removed
>>>>> entirely?
>>>>>
>>>>> --
>>>>> Ciao
>>>>> Stephan
>>>>
>>>>--
>>>>To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
>>>>the body of a message to majordomo@vger.kernel.org
>>>>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>
>>> Ciao
>>> Stephan
>
>
> Ciao
> Stephan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] crypto: algif_hash - Only export and import on sockets with data
  2015-11-02  3:12                           ` Herbert Xu
@ 2015-11-05  7:42                             ` Harsh Jain
  2015-11-05 12:19                               ` Herbert Xu
  0 siblings, 1 reply; 19+ messages in thread
From: Harsh Jain @ 2015-11-05  7:42 UTC (permalink / raw
  To: Herbert Xu; +Cc: Stephan Mueller, linux-crypto

Hi herbert,

Which kernel versions will have this patch?

Regards
Harsh Jain

On Mon, Nov 2, 2015 at 8:42 AM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Sun, Nov 01, 2015 at 12:07:12PM +0100, Stephan Mueller wrote:
>> Am Sonntag, 1. November 2015, 17:11:19 schrieb Herbert Xu:
>>
>> Hi Herbert,
>>
>> > On Sat, Oct 31, 2015 at 12:45:47AM +0100, Stephan Mueller wrote:
>> > > Testing complete: patch solves the oops and allows to successfully perform
>> > > HMAC even when having subsequent accepts and operating on those subsequent
>> > > accepts.
>> >
>> > Thanks Stephan!
>> >
>> > Unfortunately my patch is incomplete as some other thread could
>> > change ctx->more while we're in the middle of the accept call.
>> >
>> > So here is an updated version.
>>
>> Shouldn't that patch to into stable (and hopefully into 4.3) as well?
>
> Yes it is going into stable.
>
> 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] 19+ messages in thread

* Re: [PATCH v2] crypto: algif_hash - Only export and import on sockets with data
  2015-11-05  7:42                             ` Harsh Jain
@ 2015-11-05 12:19                               ` Herbert Xu
  0 siblings, 0 replies; 19+ messages in thread
From: Herbert Xu @ 2015-11-05 12:19 UTC (permalink / raw
  To: Harsh Jain; +Cc: Stephan Mueller, linux-crypto

On Thu, Nov 05, 2015 at 01:12:04PM +0530, Harsh Jain wrote:
> Hi herbert,
> 
> Which kernel versions will have this patch?

It has to go into 4.4 first before it gets backported.

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] 19+ messages in thread

end of thread, other threads:[~2015-11-05 12:19 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-25  6:26 kernel tainted while exporting shash context using af_alg interface Harsh Jain
2015-10-25 11:58 ` Stephan Mueller
2015-10-26  6:19   ` Harsh Jain
2015-10-26  9:21     ` Harsh Jain
2015-10-28  0:09       ` Stephan Mueller
2015-10-28  0:55         ` Stephan Mueller
2015-10-28 10:54           ` Harsh Jain
2015-10-28 11:23             ` Stephan Mueller
2015-10-30  8:32               ` Harsh Jain
2015-10-30 11:10                 ` Stephan Mueller
2015-10-30 12:16                   ` crypto: algif_hash - Only export and import on sockets with data Herbert Xu
2015-10-30 23:45                     ` Stephan Mueller
2015-11-01  9:11                       ` [PATCH v2] " Herbert Xu
2015-11-01 11:06                         ` Stephan Mueller
2015-11-01 11:07                         ` Stephan Mueller
2015-11-02  3:12                           ` Herbert Xu
2015-11-05  7:42                             ` Harsh Jain
2015-11-05 12:19                               ` Herbert Xu
2015-11-02  5:48                   ` kernel tainted while exporting shash context using af_alg interface Harsh Jain

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.