Re: [PATCH] crypto: DRBG - guard uninstantion by lock

[Stephan Mueller]

Am Montag, 9. April 2018, 00:46:03 CEST schrieb Theodore Y. Ts'o:

Hi Theodore,
> 
> So the syzbot will run while the patch goes through the normal e-mail
> review process, which is kind of neat.  :-)

Thank you very much for the hint. That is a neat feature indeed.

As I came late to the party and I missed the original mails, I am wondering 
about which GIT repo was used and which branch of it. With that, I would be 
happy to resubmit with the test line.

Ciao
Stephan

Re: [RFC PATCH] crypto: pcrypt - forbid recursive instantiation

[Eric Biggers]

On Fri, Mar 23, 2018 at 08:21:52AM +0800, Herbert Xu wrote:
> On Sat, Mar 10, 2018 at 03:22:31PM -0800, Eric Biggers wrote:
> > From: Eric Biggers 
> > 
> > If the pcrypt template is used multiple times in an algorithm, then a
> > deadlock occurs because all pcrypt instances share the same
> > padata_instance, which completes requests in the order submitted.  That
> > is, the inner pcrypt request waits for the outer pcrypt request while
> > the outer request is already waiting for the inner.
> > 
> > Fix this by making pcrypt forbid instantiation if pcrypt appears in the
> > underlying ->cra_driver_name.  This is somewhat of a hack, but it's a
> > simple fix that should be sufficient to prevent the deadlock.
> 
> I'm a bit uneasy with this fix.  What if pcrypt is used in the
> underlying algorithm as a fallback? Wouldn't it still dead-lock
> without triggering this check?
> 

Yep, I think you're right.

Steffen, how feasible do you think would it be to make each pcrypt instance use
its own padata instance, so different pcrypt instances aren't ordered with
respect to each other?  Or do you think there is a better solution?  This really
needs to be fixed; syzbot has hit it over 11000 times already.

Eric

Re: INFO: task hung in exit_aio

[Eric Biggers]

[+Cc linux-crypto]

On Sun, Dec 10, 2017 at 05:33:01AM -0800, syzbot wrote:
> Hello,
> 
> syzkaller hit the following crash on
> 82bcf1def3b5f1251177ad47c44f7e17af039b4b
> git://git.cmpxchg.org/linux-mmots.git/master
> compiler: gcc (GCC) 7.1.1 20170620
> .config is attached
> Raw console output is attached.
> C reproducer is attached
> syzkaller reproducer is attached. See https://goo.gl/kgGztJ
> for information about syzkaller reproducers
> 
> 
> INFO: task syzkaller265413:3189 blocked for more than 120 seconds.
>   Not tainted 4.15.0-rc2-mm1+ #39
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> syzkaller265413 D24320  3189   3150 0x8000
> Call Trace:
>  context_switch kernel/sched/core.c:2800 [inline]
>  __schedule+0x8eb/0x2060 kernel/sched/core.c:3376
>  schedule+0xf5/0x430 kernel/sched/core.c:3435
>  schedule_timeout+0x43a/0x560 kernel/time/timer.c:1776
>  do_wait_for_common kernel/sched/completion.c:91 [inline]
>  __wait_for_common kernel/sched/completion.c:112 [inline]
>  wait_for_common kernel/sched/completion.c:123 [inline]
>  wait_for_completion+0x44b/0x7b0 kernel/sched/completion.c:144
>  exit_aio+0x47f/0x530 fs/aio.c:903
>  __mmput kernel/fork.c:968 [inline]
>  mmput+0x1b1/0x6c0 kernel/fork.c:992
>  exit_mm kernel/exit.c:544 [inline]
>  do_exit+0x90a/0x1ae0 kernel/exit.c:856
>  do_group_exit+0x149/0x400 kernel/exit.c:972
>  SYSC_exit_group kernel/exit.c:983 [inline]
>  SyS_exit_group+0x1d/0x20 kernel/exit.c:981
>  entry_SYSCALL_64_fastpath+0x1f/0x96
> RIP: 0033:0x440a99
> RSP: 002b:7ffc5abebf68 EFLAGS: 0202 ORIG_RAX: 00e7
> RAX: ffda RBX:  RCX: 00440a99
> RDX: 00440a99 RSI: 0058 RDI: 
> RBP: 0001cbc8 R08: 5abec108 R09: 00401fd0
> R10: 5abec108 R11: 0202 R12: 
> R13: 00401fd0 R14:  R15: 
> 
> Showing all locks held in the system:
> 2 locks held by khungtaskd/673:
>  #0:  (rcu_read_lock){}, at: []
> check_hung_uninterruptible_tasks kernel/hung_task.c:175 [inline]
>  #0:  (rcu_read_lock){}, at: [] watchdog+0x1c5/0xd60
> kernel/hung_task.c:249
>  #1:  (tasklist_lock){.+.+}, at: [<36eecee2>]
> debug_show_all_locks+0xd3/0x400 kernel/locking/lockdep.c:4554
> 2 locks held by getty/3117:
>  #0:  (>ldisc_sem){}, at: [<8d3f9495>]
> ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
>  #1:  (>atomic_read_lock){+.+.}, at: []
> n_tty_read+0x2f2/0x1a10 drivers/tty/n_tty.c:2131
> 2 locks held by getty/3118:
>  #0:  (>ldisc_sem){}, at: [<8d3f9495>]
> ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
>  #1:  (>atomic_read_lock){+.+.}, at: []
> n_tty_read+0x2f2/0x1a10 drivers/tty/n_tty.c:2131
> 2 locks held by getty/3119:
>  #0:  (>ldisc_sem){}, at: [<8d3f9495>]
> ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
>  #1:  (>atomic_read_lock){+.+.}, at: []
> n_tty_read+0x2f2/0x1a10 drivers/tty/n_tty.c:2131
> 2 locks held by getty/3120:
>  #0:  (>ldisc_sem){}, at: [<8d3f9495>]
> ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
>  #1:  (>atomic_read_lock){+.+.}, at: []
> n_tty_read+0x2f2/0x1a10 drivers/tty/n_tty.c:2131
> 2 locks held by getty/3121:
>  #0:  (>ldisc_sem){}, at: [<8d3f9495>]
> ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
>  #1:  (>atomic_read_lock){+.+.}, at: []
> n_tty_read+0x2f2/0x1a10 drivers/tty/n_tty.c:2131
> 2 locks held by getty/3122:
>  #0:  (>ldisc_sem){}, at: [<8d3f9495>]
> ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
>  #1:  (>atomic_read_lock){+.+.}, at: []
> n_tty_read+0x2f2/0x1a10 drivers/tty/n_tty.c:2131
> 2 locks held by getty/3123:
>  #0:  (>ldisc_sem){}, at: [<8d3f9495>]
> ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
>  #1:  (>atomic_read_lock){+.+.}, at: []
> n_tty_read+0x2f2/0x1a10 drivers/tty/n_tty.c:2131
> 
> =
> 
> NMI backtrace for cpu 1
> CPU: 1 PID: 673 Comm: khungtaskd Not tainted 4.15.0-rc2-mm1+ #39
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:17 [inline]
>  dump_stack+0x194/0x257 lib/dump_stack.c:53
>  nmi_cpu_backtrace+0x1d2/0x210 lib/nmi_backtrace.c:103
>  nmi_trigger_cpumask_backtrace+0x122/0x180 lib/nmi_backtrace.c:62
>  arch_trigger_cpumask_backtrace+0x14/0x20 arch/x86/kernel/apic/hw_nmi.c:38
>  trigger_all_cpu_backtrace include/linux/nmi.h:138 [inline]
>  check_hung_task kernel/hung_task.c:132 [inline]
>  check_hung_uninterruptible_tasks kernel/hung_task.c:190 [inline]
>  watchdog+0x90c/0xd60 kernel/hung_task.c:249
>  kthread+0x37a/0x440 kernel/kthread.c:238
>  ret_from_fork+0x24/0x30 

Re: [PATCH] crypto: DRBG - guard uninstantion by lock

[Theodore Y. Ts'o]

On Sun, Apr 08, 2018 at 09:07:03PM +0200, Stephan Müller wrote:
> Can you please check whether the attached patch fixes the issue?
> 

Stephan,

FYI, if you incude in your e-mail "#syz test  " as
the first line of your patch and the syzbot e-mail is cc'ed, the
syzbot will automatically apply the patch in the e-mail against the
git tree/branch specified in the "#syz test" line, and then try to see
if the problem it discovered still reproduces --- and then send you
e-mail one way or another.

So the syzbot will run while the patch goes through the normal e-mail
review process, which is kind of neat.  :-)

Cheers,

- Ted

[PATCH] crypto: DRBG - guard uninstantion by lock

[Stephan Müller]

Am Sonntag, 8. April 2018, 17:41:17 CEST schrieb Dmitry Vyukov:

Hi Dmitry,
> 
> Hi,
> 
> Here is config and kernel commit:
> https://groups.google.com/d/msg/syzkaller-bugs/PINYyzoaG1s/ntZPOZdcCAAJ
> You can also find compiler and image here if necessary:
> https://github.com/google/syzkaller/blob/master/docs/syzbot.md
> 
> And note that the program needs to be compiled with -m32. The bugs is
> probably not-compat specific, but the program injects fault into a
> particular malloc invocation and maybe malloc numbering is affected by
> compat path.

I am unable to reproduce the issue. But since you mention that you induce 
errors, I could see that the unlocking of the DRBG context is too soon.

Can you please check whether the attached patch fixes the issue?

Thanks

---8<---

In the error code path, the uninstantiation must be guarded by a lock to
ensure that the modification of the context is fully atomic.

Signed-off-by: Stephan Mueller 
Reported-by: syzkaller
---
 crypto/drbg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/drbg.c b/crypto/drbg.c
index 4faa2781c964..68c1949a253f 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -1510,8 +1510,8 @@ static int drbg_instantiate(struct drbg_state *drbg, 
struct drbg_string *pers,
return ret;
 
 free_everything:
-   mutex_unlock(>drbg_mutex);
drbg_uninstantiate(drbg);
+   mutex_unlock(>drbg_mutex);
return ret;
 }
 
-- 
2.14.3

Re: [RFC 0/2] add integrity and security to TPM2 transactions

[Ken Goldman]

On 3/5/2018 9:04 AM, Jason Gunthorpe wrote:

On Fri, Mar 02, 2018 at 10:04:54PM -0800, James Bottomley wrote:

By now, everybody knows we have a problem with the TPM2_RS_PW easy
button on TPM2 in that transactions on the TPM bus can be intercepted
and altered.  The way to fix this is to use real sessions for HMAC
capabilities to ensure integrity and to use parameter and response
encryption to ensure confidentiality of the data flowing over the TPM
bus.


We have the same issue for TPM1 then right?


TPM 1.2 did not have the concept of plaintext password sessions.  If 
authorization was used, it was always with an HMAC.

[PATCH] AF_ALG: register completely initialized request in list

[Stephan Müller]

Hi,

May I ask to check whether this patch fixes the issue? I cannot re-create
the issue with the reproducter. Yet, as far as I understand, you try to
induce errors which shall validate whether the error code paths are correct.

The fix below should ensure this now.

Thanks a lot.

---8<---

>From 8f083e7b0684a9f91c186d7b46eec34e439689c3 Mon Sep 17 00:00:00 2001
From: Stephan Mueller 
Date: Sun, 8 Apr 2018 19:53:59 +0200
Subject: [PATCH] AF_ALG: Initialize sg_num_bytes in error code path

The RX SGL in processing is already registered with the RX SGL tracking
list to support proper cleanup. The cleanup code path uses the
sg_num_bytes variable which must therefore be always initialized, even
in the error code path.

Signed-off-by: Stephan Mueller 
Reported-by: syzbot+9c251bdd09f83b92b...@syzkaller.appspotmail.com
---
 crypto/af_alg.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index c49766b03165..0d555c072669 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -1156,8 +1156,10 @@ int af_alg_get_rsgl(struct sock *sk, struct msghdr *msg, 
int flags,
 
/* make one iovec available as scatterlist */
err = af_alg_make_sg(>sgl, >msg_iter, seglen);
-   if (err < 0)
+   if (err < 0) {
+   rsgl->sg_num_bytes = 0;
return err;
+   }
 
/* chain the new scatterlist with previous one */
if (areq->last_rsgl)
-- 
2.14.3

Re: WARNING in kmem_cache_free

[Dmitry Vyukov]

On Sun, Apr 8, 2018 at 5:31 PM, Stephan Müller  wrote:
> Am Sonntag, 8. April 2018, 13:18:06 CEST schrieb Dmitry Vyukov:
>
> Hi Dmitry,
>
>>
>> Running syz-repro utility on this log, I think I've found the guilty guy:
>> https://gist.githubusercontent.com/dvyukov/1dd75d55efd238e7207af1cc38478b3a/
>> raw/403859b56b161a6fbb158e8953fac5bb6e73b1a1/gistfile1.txt
>>
>
> I am unable to reproduce it with the code. I am using the current
> cryptodev-2.6 tree with kazan enabled. Could you please give me your kernel
> config or a pointer of the used tree?

Hi,

Here is config and kernel commit:
https://groups.google.com/d/msg/syzkaller-bugs/PINYyzoaG1s/ntZPOZdcCAAJ
You can also find compiler and image here if necessary:
https://github.com/google/syzkaller/blob/master/docs/syzbot.md

And note that the program needs to be compiled with -m32. The bugs is
probably not-compat specific, but the program injects fault into a
particular malloc invocation and maybe malloc numbering is affected by
compat path.


>> It crashes as:
>> BUG: KASAN: use-after-free in drbg_kcapi_seed+0x1178/0x12e0
>> and:
>> BUG: unable to handle kernel paging request at ebe00020
>> and with other indications of badly corrupted heap.
>>
>> This points to crypto/drbg.c, so +crypto maintainers.
>
>
> Ciao
> Stephan
>
>
> --
> You received this message because you are subscribed to the Google Groups 
> "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to syzkaller-bugs+unsubscr...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/syzkaller-bugs/4564679.HlOejCIXXz%40positron.chronox.de.
> For more options, visit https://groups.google.com/d/optout.

Re: WARNING in kmem_cache_free

[Stephan Müller]

Am Sonntag, 8. April 2018, 13:18:06 CEST schrieb Dmitry Vyukov:

Hi Dmitry,

> 
> Running syz-repro utility on this log, I think I've found the guilty guy:
> https://gist.githubusercontent.com/dvyukov/1dd75d55efd238e7207af1cc38478b3a/
> raw/403859b56b161a6fbb158e8953fac5bb6e73b1a1/gistfile1.txt
> 

I am unable to reproduce it with the code. I am using the current 
cryptodev-2.6 tree with kazan enabled. Could you please give me your kernel 
config or a pointer of the used tree?

> It crashes as:
> BUG: KASAN: use-after-free in drbg_kcapi_seed+0x1178/0x12e0
> and:
> BUG: unable to handle kernel paging request at ebe00020
> and with other indications of badly corrupted heap.
> 
> This points to crypto/drbg.c, so +crypto maintainers.


Ciao
Stephan

Re: WARNING in kmem_cache_free

[Dmitry Vyukov]

On Sun, Apr 8, 2018 at 12:26 PM, Dmitry Vyukov  wrote:
> On Sun, Apr 8, 2018 at 8:01 AM, Matthew Wilcox  wrote:
>> On Fri, Apr 06, 2018 at 03:33:36PM +0200, Dmitry Vyukov wrote:
>>> On Fri, Apr 6, 2018 at 3:24 PM, syzbot
>>>  wrote:
>>> > Unfortunately, I don't have any reproducer for this crash yet.
>>>
>>> Interesting type of bug, I think we see this for the first time.
>>
>> Can you focus syzbot to try to find a reproducer?  This seems to be
>> produced by calling mount() with a pathname that's somewhere between,
>> say, 3950 & 4100 bytes long from a compat 32-bit task.
>
>
> Something in the log definitely triggers a very bad heap corruption.
>
> This can be reproduced following instructions at:
> https://github.com/google/syzkaller/blob/master/docs/syzbot.md#syzkaller-reproducers
>
> and then running:
> ./syz-execprog -sandbox=namespace -arch=386 -repeat=0 -procs=10 log.txt
>
> where log.txt comes from "Raw console output" link.
>
> Note that you need to build syzkaller with 'make TARGETARCH=386' and
> the use bin/linux_386/syz-executor.
>
> While running it I got:
> BUG: KASAN: double-free or invalid-free in free_request_size+0x5b/0x70
> block/blk-core.c:769
> https://gist.githubusercontent.com/dvyukov/05f4e77a34795d329aa7a2f40265e396/raw/63a29123b79f1fbad3521d0ff034946be68bfd4a/gistfile1.txt
>
> Then kernel BUG at mm/slab.c:4407!
> https://gist.githubusercontent.com/dvyukov/5b3bcc90d326e9da3636aea2c95ace8f/raw/1589504c708994936681d61ba9d70029998b9b1a/gistfile1.txt
>
> And then BUG: unable to handle kernel paging request at ebe00020
> https://gist.githubusercontent.com/dvyukov/72025b1c68e488f4fda243e0c152f044/raw/d2c171bc55ad3a43cea33095fa2eea48768b1131/gistfile1.txt
>
> One interesting thing is that if I run the log once and it does not
> crash, then when I try to start binary again I am getting:
> [  456.837870] Invalid argument reading file caps for /root/syz-executor
> The binary somehow becomes broken on disk...
>
> I guess syzbot did find a reproducer in this log, but did not
> attribute it to this bug as it causes crashes all over the place.



Running syz-repro utility on this log, I think I've found the guilty guy:
https://gist.githubusercontent.com/dvyukov/1dd75d55efd238e7207af1cc38478b3a/raw/403859b56b161a6fbb158e8953fac5bb6e73b1a1/gistfile1.txt

It crashes as:
BUG: KASAN: use-after-free in drbg_kcapi_seed+0x1178/0x12e0
and:
BUG: unable to handle kernel paging request at ebe00020
and with other indications of badly corrupted heap.

This points to crypto/drbg.c, so +crypto maintainers.

Re: [PATCH 3/6] crypto: api - avoid VLA use

[Herbert Xu]

On Sun, Apr 08, 2018 at 11:07:12AM +0200, Salvatore Mesoraca wrote:
>
> > This check should be done when the algorithm is registered.  Perhaps
> > crypto_check_alg.
> 
> Please correct me if I'm wrong:
> isn't crypto_check_alg invoked also during hashing algorithm registration?
> In this patch-set I'm dealing only with ciphers, because the maximum
> block size (16)
> is relatively small and it's also the most common block size with
> ciphers (maybe I should
> have explicitly referenced ciphers in the macro names, my bad).
> I don't think that it would be OK to use a similar approach for hashes
> too, because some
> of them have block size >= 1024 bytes.

Yes we want to make it for ciphers only even if we move it to
crypto_check_alg.

For a legacy type like cipher cou can do it by

if (!alg->cra_type && (alg->cra_flags & CRYPTO_ALG_TYPE_MASK) ==
  CRYPTO_ALG_TYPE_CIPHER)
do_cipher_specific_check();

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 2/6] crypto: ctr - avoid VLA use

[Herbert Xu]

On Sun, Apr 08, 2018 at 10:58:48AM +0200, Salvatore Mesoraca wrote:
>
> Fair enough.
> After removing the individual checks the modification to the single files
> will be just a couple of lines, is it OK for you if I collapse all of them in
> just a single commit?

Sure.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 3/6] crypto: api - avoid VLA use

[Salvatore Mesoraca]

2018-04-08 5:16 GMT+02:00 Herbert Xu :
> On Sat, Apr 07, 2018 at 08:38:20PM +0200, Salvatore Mesoraca wrote:
>>
>>  int crypto_init_cipher_ops(struct crypto_tfm *tfm)
>>  {
>> + const unsigned long alignmask = crypto_tfm_alg_alignmask(tfm);
>> + const unsigned int size = crypto_tfm_alg_blocksize(tfm);
>>   struct cipher_tfm *ops = >crt_cipher;
>>   struct cipher_alg *cipher = >__crt_alg->cra_cipher;
>>
>> + if (size > MAX_BLOCKSIZE || alignmask > MAX_ALIGNMASK)
>> + return -EINVAL;
>> +
>
> This check should be done when the algorithm is registered.  Perhaps
> crypto_check_alg.

Please correct me if I'm wrong:
isn't crypto_check_alg invoked also during hashing algorithm registration?
In this patch-set I'm dealing only with ciphers, because the maximum
block size (16)
is relatively small and it's also the most common block size with
ciphers (maybe I should
have explicitly referenced ciphers in the macro names, my bad).
I don't think that it would be OK to use a similar approach for hashes
too, because some
of them have block size >= 1024 bytes.

Thank you for your time,

Salvatore

Re: [PATCH 2/6] crypto: ctr - avoid VLA use

[Salvatore Mesoraca]

018-04-08 5:19 GMT+02:00 Herbert Xu :
> On Sat, Apr 07, 2018 at 08:38:19PM +0200, Salvatore Mesoraca wrote:
>>
>> @@ -206,6 +207,14 @@ static struct crypto_instance *crypto_ctr_alloc(struct 
>> rtattr **tb)
>>   if (alg->cra_blocksize < 4)
>>   goto out_put_alg;
>>
>> + /* Block size must be <= MAX_BLOCKSIZE. */
>> + if (alg->cra_blocksize > MAX_BLOCKSIZE)
>> + goto out_put_alg;
>> +
>> + /* Alignmask must be <= MAX_ALIGNMASK. */
>> + if (alg->cra_alignmask > MAX_ALIGNMASK)
>> + goto out_put_alg;
>> +
>
> Since you're also adding a check to cipher algorithms in general,
> none of these individual checks are needed anymore.

Fair enough.
After removing the individual checks the modification to the single files
will be just a couple of lines, is it OK for you if I collapse all of them in
just a single commit?

Thank you,

Salvatore

Re: [PATCH 1/6] crypto: api - laying macros for statically allocated buffers

[Salvatore Mesoraca]

2018-04-08 5:15 GMT+02:00 Herbert Xu :
> On Sat, Apr 07, 2018 at 08:38:18PM +0200, Salvatore Mesoraca wrote:
>> Creating 2 new compile-time constants for internal use,
>> in preparation for the removal of VLAs[1] from crypto code.
>> All ciphers implemented in Linux have a block size less than or
>> equal to 16 bytes and the most demanding hw require 16 bytes
>> alignment for the block buffer.
>>
>> [1] 
>> http://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com
>>
>> Signed-off-by: Salvatore Mesoraca 
>> ---
>>  crypto/internal.h | 8 
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/crypto/internal.h b/crypto/internal.h
>> index 9a3f399..89ae41e 100644
>> --- a/crypto/internal.h
>> +++ b/crypto/internal.h
>> @@ -26,6 +26,14 @@
>>  #include 
>>  #include 
>>
>> +/*
>> + * Maximum values for blocksize and alignmask, used to allocate
>> + * static buffers that are big enough for any combination of
>> + * ciphers and architectures.
>> + */
>> +#define MAX_BLOCKSIZE16
>> +#define MAX_ALIGNMASK15
>
> No please don't put this here if you intend on using it everywhere.
> This file is reserved for truly internal bits.
>
> Perhaps include/crypto/algapi.h would be a better place.

OK, thank you for the suggestion :)

Salvatore

Re: [PATCH 0/6] Remove several VLAs in the crypto subsystem

[Salvatore Mesoraca]

2018-04-07 21:56 GMT+02:00 Kees Cook :
> On Sat, Apr 7, 2018 at 11:38 AM, Salvatore Mesoraca
>  wrote:
>> As suggested by Laura Abbott[2], I'm resending my patch with
>> MAX_BLOCKSIZE and MAX_ALIGNMASK defined in an header, so they
>> can be used in other places.
>> I take this opportuinuty to deal with some other VLAs not
>> handled in the old patch.
>>
>> [1] 
>> http://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com
>> [2] http://lkml.kernel.org/r/4e536889-439a-49e6-dd95-2d4286913...@redhat.com
>>
>> Salvatore Mesoraca (6):
>>   crypto: api - laying macros for statically allocated buffers
>>   crypto: ctr - avoid VLA use
>>   crypto: api - avoid VLA use
>>   crypto: pcbc - avoid VLA use
>>   crypto: cts - avoid VLA use
>>   crypto: cfb - avoid VLA use
>>
>>  crypto/cfb.c  | 14 ++
>>  crypto/cipher.c   |  7 ++-
>>  crypto/ctr.c  | 13 +++--
>>  crypto/cts.c  |  8 ++--
>>  crypto/internal.h |  8 
>>  crypto/pcbc.c |  9 +++--
>>  6 files changed, 48 insertions(+), 11 deletions(-)
>
> These all look good to me! Thanks for the refactoring. :)
>
> Reviewed-by: Kees Cook 

Thank you!

Salvatore

KMSAN: uninit-value in af_alg_free_areq_sgls

[syzbot]

Hello,

syzbot hit the following crash on  
https://github.com/google/kmsan.git/master commit

e2ab7e8abba47a2f2698216258e5d8727ae58717 (Fri Apr 6 16:24:31 2018 +)
kmsan: temporarily disable visitAsmInstruction() to help syzbot
syzbot dashboard link:  
https://syzkaller.appspot.com/bug?extid=9c251bdd09f83b92ba95


So far this crash happened 11 times on  
https://github.com/google/kmsan.git/master.

C reproducer: https://syzkaller.appspot.com/x/repro.c?id=5551473324720128
syzkaller reproducer:  
https://syzkaller.appspot.com/x/repro.syz?id=4782073151750144
Raw console output:  
https://syzkaller.appspot.com/x/log.txt?id=5003160619843584
Kernel config:  
https://syzkaller.appspot.com/x/.config?id=6627248707860932248

compiler: clang version 7.0.0 (trunk 329391)

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+9c251bdd09f83b92b...@syzkaller.appspotmail.com
It will help syzbot understand when the bug is fixed. See footer for  
details.

If you forward the report, please keep this part and the footer.

==
BUG: KMSAN: uninit-value in atomic_sub arch/x86/include/asm/atomic.h:65  
[inline]
BUG: KMSAN: uninit-value in af_alg_free_areq_sgls+0x5ff/0xb20  
crypto/af_alg.c:669

CPU: 1 PID: 3568 Comm: syzkaller909997 Not tainted 4.16.0+ #82
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011

Call Trace:
 __dump_stack lib/dump_stack.c:17 [inline]
 dump_stack+0x185/0x1d0 lib/dump_stack.c:53
 kmsan_report+0x142/0x240 mm/kmsan/kmsan.c:1067
 __msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:676
 atomic_sub arch/x86/include/asm/atomic.h:65 [inline]
 af_alg_free_areq_sgls+0x5ff/0xb20 crypto/af_alg.c:669
 af_alg_free_resources+0x66/0xf0 crypto/af_alg.c:1033
 _aead_recvmsg crypto/algif_aead.c:321 [inline]
 aead_recvmsg+0x9a4/0x2960 crypto/algif_aead.c:334
 aead_recvmsg_nokey+0x129/0x160 crypto/algif_aead.c:452
 sock_recvmsg_nosec net/socket.c:803 [inline]
 sock_recvmsg+0x1d0/0x230 net/socket.c:810
 ___sys_recvmsg+0x3fb/0x810 net/socket.c:2205
 __sys_recvmsg net/socket.c:2250 [inline]
 SYSC_recvmsg+0x298/0x3c0 net/socket.c:2262
 SyS_recvmsg+0x54/0x80 net/socket.c:2257
 do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
 entry_SYSCALL_64_after_hwframe+0x3d/0xa2
RIP: 0033:0x43ff29
RSP: 002b:7ffd9919c808 EFLAGS: 0207 ORIG_RAX: 002f
RAX: ffda RBX: 004002c8 RCX: 0043ff29
RDX:  RSI: 2040 RDI: 0004
RBP: 006ca018 R08: 004002c8 R09: 004002c8
R10: 004002c8 R11: 0207 R12: 00401850
R13: 004018e0 R14:  R15: 

Uninit was created at:
 kmsan_save_stack_with_flags mm/kmsan/kmsan.c:278 [inline]
 kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:188
 kmsan_kmalloc+0x94/0x100 mm/kmsan/kmsan.c:314
 __kmalloc+0x23c/0x350 mm/slub.c:3791
 kmalloc include/linux/slab.h:517 [inline]
 sock_kmalloc+0x14e/0x270 net/core/sock.c:1986
 af_alg_get_rsgl+0x427/0xe10 crypto/af_alg.c:1149
 _aead_recvmsg crypto/algif_aead.c:163 [inline]
 aead_recvmsg+0x953/0x2960 crypto/algif_aead.c:334
 aead_recvmsg_nokey+0x129/0x160 crypto/algif_aead.c:452
 sock_recvmsg_nosec net/socket.c:803 [inline]
 sock_recvmsg+0x1d0/0x230 net/socket.c:810
 ___sys_recvmsg+0x3fb/0x810 net/socket.c:2205
 __sys_recvmsg net/socket.c:2250 [inline]
 SYSC_recvmsg+0x298/0x3c0 net/socket.c:2262
 SyS_recvmsg+0x54/0x80 net/socket.c:2257
 do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
 entry_SYSCALL_64_after_hwframe+0x3d/0xa2
==


---
This bug is generated by a dumb bot. It may contain errors.
See https://goo.gl/tpsmEJ for details.
Direct all questions to syzkal...@googlegroups.com.

syzbot will keep track of this bug report.
If you forgot to add the Reported-by tag, once the fix for this bug is  
merged

into any tree, please reply to this email with:
#syz fix: exact-commit-title
If you want to test a patch for this bug, please reply with:
#syz test: git://repo/address.git branch
and provide the patch inline or as an attachment.
To mark this as a duplicate of another syzbot report, please reply with:
#syz dup: exact-subject-of-another-report
If it's a one-off invalid bug report, please reply with:
#syz invalid
Note: if the crash happens again, it will cause creation of a new bug  
report.

Note: all commands must start from beginning of the line in the email body.