KASAN: use-after-free Read in __list_del_entry_valid (2)

[syzbot]

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


==================================================================
BUG: KASAN: use-after-free in __list_del_entry_valid+0x11d/0x150
lib/list_debug.c:42
Read of size 8 at addr ffff8801c4471248 by task syzkaller960526/6767

CPU: 0 PID: 6767 Comm: syzkaller960526 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
print_address_description+0x73/0x250 mm/kasan/report.c:252
kasan_report_error mm/kasan/report.c:351 [inline]
kasan_report+0x25b/0x340 mm/kasan/report.c:409
__asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:430
__list_del_entry_valid+0x11d/0x150 lib/list_debug.c:42
__list_del_entry include/linux/list.h:117 [inline]
list_del include/linux/list.h:125 [inline]
crypto_larval_kill+0x79/0x2e0 crypto/api.c:164
crypto_alg_mod_lookup+0x178/0x1b0 crypto/api.c:283
crypto_find_alg crypto/api.c:501 [inline]
crypto_alloc_tfm+0xf3/0x2f0 crypto/api.c:534
crypto_alloc_aead+0x2c/0x40 crypto/aead.c:342
aead_bind+0x70/0x140 crypto/algif_aead.c:482
alg_bind+0x1ab/0x440 crypto/af_alg.c:179
SYSC_bind+0x1b4/0x3f0 net/socket.c:1454
SyS_bind+0x24/0x30 net/socket.c:1440
entry_SYSCALL_64_fastpath+0x1f/0x96
RIP: 0033:0x4406f9
RSP: 002b:00007ffe2d384878 EFLAGS: 00000203 ORIG_RAX: 0000000000000031
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00000000004406f9
RDX: 0000000000000058 RSI: 0000000020269000 RDI: 0000000000000003
RBP: 000000000000ac2d R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000203 R12: 0000000000000000
R13: 0000000000401c30 R14: 0000000000000000 R15: 0000000000000000

Allocated by task 6767:
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459 [inline]
kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:551
kmem_cache_alloc_trace+0x136/0x750 mm/slab.c:3614
kmalloc include/linux/slab.h:516 [inline]
kzalloc include/linux/slab.h:705 [inline]
crypto_larval_alloc+0x51/0x1e0 crypto/api.c:114
crypto_larval_add crypto/api.c:136 [inline]
crypto_larval_lookup.part.8+0x194/0x400 crypto/api.c:232
crypto_larval_lookup crypto/api.c:212 [inline]
crypto_alg_mod_lookup+0x77/0x1b0 crypto/api.c:271
crypto_find_alg crypto/api.c:501 [inline]
crypto_alloc_tfm+0xf3/0x2f0 crypto/api.c:534
crypto_alloc_aead+0x2c/0x40 crypto/aead.c:342
aead_bind+0x70/0x140 crypto/algif_aead.c:482
alg_bind+0x1ab/0x440 crypto/af_alg.c:179
SYSC_bind+0x1b4/0x3f0 net/socket.c:1454
SyS_bind+0x24/0x30 net/socket.c:1440
entry_SYSCALL_64_fastpath+0x1f/0x96

Freed by task 3157:
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459 [inline]
kasan_slab_free+0x71/0xc0 mm/kasan/kasan.c:524
__cache_free mm/slab.c:3492 [inline]
kfree+0xca/0x250 mm/slab.c:3807
crypto_larval_destroy+0x110/0x150 crypto/api.c:107
crypto_alg_put crypto/internal.h:116 [inline]
crypto_larval_kill+0x1e8/0x2e0 crypto/api.c:167
crypto_wait_for_test+0x87/0xb0 crypto/algapi.c:351
crypto_register_instance+0x2bf/0x430 crypto/algapi.c:558
aead_register_instance+0x161/0x1c0 crypto/aead.c:421
pcrypt_create_aead crypto/pcrypt.c:322 [inline]
pcrypt_create+0x542/0x6c0 crypto/pcrypt.c:346
cryptomgr_probe+0x74/0x240 crypto/algboss.c:75
kthread+0x37a/0x440 kernel/kthread.c:238
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:524

The buggy address belongs to the object at ffff8801c4471240
which belongs to the cache kmalloc-1024 of size 1024
The buggy address is located 8 bytes inside of
1024-byte region [ffff8801c4471240, ffff8801c4471640)
The buggy address belongs to the page:
page:000000007b38bfe6 count:1 mapcount:0 mapping:000000009d8bf81b index:0x0
compound_mapcount: 0
flags: 0x2fffc0000008100(slab|head)
raw: 02fffc0000008100 ffff8801c4470040 0000000000000000 0000000100000007
raw: ffffea000713f920 ffffea000711a120 ffff8801dac00ac0 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
ffff8801c4471100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8801c4471180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff8801c4471200: fb fb fb fb fb fb fb fb fb fb fb fb 00 00 00 00
^
ffff8801c4471280: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff8801c4471300: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
==================================================================


---
This bug is generated by a dumb bot. It may contain errors.
See https://goo.gl/tpsmEJ for details.
Direct all questions to syzk...@googlegroups.com.
Please credit me with: Reported-by: syzbot <syzk...@googlegroups.com>

syzbot will keep track of this bug report.
Once a fix for this bug is merged into any tree, 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.

[Stephan Mueller]

Am Montag, 18. Dezember 2017, 06:50:01 CET schrieb syzbot:

Hi,

This bug seems to be triggerable again by the fact that some strange mask/type
combo is used. When setting it to zero, there is no crash.

Therefore I would see that this issue would be covered with the fix that we
currently work on to limit the number of allowed mask/type values.

Ciao
Stephan

[Eric Biggers]

From: Eric Biggers <ebig...@google.com>

pcrypt is using the old way of freeing instances, where the ->free()
method specified in the 'struct crypto_template' is passed a pointer to
the 'struct crypto_instance'. But the crypto_instance is being
kfree()'d directly, which is incorrect because the memory was actually
allocated as an aead_instance, which contains the crypto_instance at a
nonzero offset. Thus, the wrong pointer was being kfree()'d.

Fix it by switching to the new way to free aead_instance's where the
->free() method is specified in the aead_instance itself.

Reported-by: syzbot <syzk...@googlegroups.com>
Fixes: 0496f56065e0 ("crypto: pcrypt - Add support for new AEAD interface")
Cc: <sta...@vger.kernel.org> # v4.2+
Signed-off-by: Eric Biggers <ebig...@google.com>
---
crypto/pcrypt.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c
index ee9cfb99fe25..f8ec3d4ba4a8 100644
--- a/crypto/pcrypt.c
+++ b/crypto/pcrypt.c
@@ -254,6 +254,14 @@ static void pcrypt_aead_exit_tfm(struct crypto_aead *tfm)
crypto_free_aead(ctx->child);
}

+static void pcrypt_free(struct aead_instance *inst)
+{
+ struct pcrypt_instance_ctx *ctx = aead_instance_ctx(inst);
+
+ crypto_drop_aead(&ctx->spawn);
+ kfree(inst);
+}
+
static int pcrypt_init_instance(struct crypto_instance *inst,
struct crypto_alg *alg)
{
@@ -319,6 +327,8 @@ static int pcrypt_create_aead(struct crypto_template *tmpl, struct rtattr **tb,
inst->alg.encrypt = pcrypt_aead_encrypt;
inst->alg.decrypt = pcrypt_aead_decrypt;

+ inst->free = pcrypt_free;
+
err = aead_register_instance(tmpl, inst);
if (err)
goto out_drop_aead;
@@ -349,14 +359,6 @@ static int pcrypt_create(struct crypto_template *tmpl, struct rtattr **tb)
return -EINVAL;
}

-static void pcrypt_free(struct crypto_instance *inst)
-{
- struct pcrypt_instance_ctx *ctx = crypto_instance_ctx(inst);
-
- crypto_drop_aead(&ctx->spawn);
- kfree(inst);
-}
-
static int pcrypt_cpumask_change_notify(struct notifier_block *self,
unsigned long val, void *data)
{
@@ -469,7 +471,6 @@ static void pcrypt_fini_padata(struct padata_pcrypt *pcrypt)
static struct crypto_template pcrypt_tmpl = {
.name = "pcrypt",
.create = pcrypt_create,
- .free = pcrypt_free,
.module = THIS_MODULE,
};

--
2.15.1.620.gb9897f4670-goog

[Dmitry Vyukov]

On Wed, Dec 20, 2017 at 11:28 PM, Eric Biggers <ebig...@gmail.com> wrote:
> From: Eric Biggers <ebig...@google.com>
>
> pcrypt is using the old way of freeing instances, where the ->free()
> method specified in the 'struct crypto_template' is passed a pointer to
> the 'struct crypto_instance'. But the crypto_instance is being
> kfree()'d directly, which is incorrect because the memory was actually
> allocated as an aead_instance, which contains the crypto_instance at a
> nonzero offset. Thus, the wrong pointer was being kfree()'d.

That's interesting. KASAN does not detect frees of invalid pointers
(e.g. into a middle of an object). It should.
I've requested a component for KASAN in kernel bugzilla to file this
(not sure if anybody is actually reading these emails), and so far
filed it in an internal bug tracker.

> --
> 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-bug...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20171220222825.207321-1-ebiggers3%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.

[Herbert Xu]

On Wed, Dec 20, 2017 at 02:28:25PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebig...@google.com>
>
> pcrypt is using the old way of freeing instances, where the ->free()
> method specified in the 'struct crypto_template' is passed a pointer to
> the 'struct crypto_instance'. But the crypto_instance is being
> kfree()'d directly, which is incorrect because the memory was actually
> allocated as an aead_instance, which contains the crypto_instance at a
> nonzero offset. Thus, the wrong pointer was being kfree()'d.
>
> Fix it by switching to the new way to free aead_instance's where the
> ->free() method is specified in the aead_instance itself.
>
> Reported-by: syzbot <syzk...@googlegroups.com>
> Fixes: 0496f56065e0 ("crypto: pcrypt - Add support for new AEAD interface")
> Cc: <sta...@vger.kernel.org> # v4.2+
> Signed-off-by: Eric Biggers <ebig...@google.com>

Patch applied. Thanks.
--
Email: Herbert Xu <her...@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[Eric Biggers]

On Sun, Dec 17, 2017 at 09:50:01PM -0800, syzbot wrote:
>
> syzbot will keep track of this bug report.
> Once a fix for this bug is merged into any tree, 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.

#syz fix: crypto: pcrypt - fix freeing pcrypt instances