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 +0000)
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+9c251b...@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:00007ffd9919c808 EFLAGS: 00000207 ORIG_RAX: 000000000000002f
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 000000000043ff29
RDX: 0000000000000000 RSI: 0000000020000040 RDI: 0000000000000004
RBP: 00000000006ca018 R08: 00000000004002c8 R09: 00000000004002c8
R10: 00000000004002c8 R11: 0000000000000207 R12: 0000000000401850
R13: 00000000004018e0 R14: 0000000000000000 R15: 0000000000000000

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 syzk...@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.

[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 <smue...@chronox.de>
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 <smue...@chronox.de>
Reported-by: syzbot+9c251b...@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(&rsgl->sgl, &msg->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

[Dmitry Vyukov]

On Sun, Apr 8, 2018 at 7:57 PM, Stephan Müller <smue...@chronox.de> wrote:
> 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.

You can ask syzbot to test by replying to its report email with a test
command, see:
https://github.com/google/syzkaller/blob/master/docs/syzbot.md#communication-with-syzbot

Note that all testing of KMSAN bugs needs to go to KMSAN tree, for details see:
https://github.com/google/syzkaller/blob/master/docs/syzbot.md#kmsan-bugs

> --
> 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/3337259.MW9pfDCdka%40positron.chronox.de.
> For more options, visit https://groups.google.com/d/optout.

[Stephan Mueller]

Am Montag, 9. April 2018, 09:51:13 CEST schrieb Dmitry Vyukov:

Hi Dmitry,

> You can ask syzbot to test by replying to its report email with a test
> command, see:
> https://github.com/google/syzkaller/blob/master/docs/syzbot.md#communication
> -with-syzbot
>
> Note that all testing of KMSAN bugs needs to go to KMSAN tree, for details
> see:
> https://github.com/google/syzkaller/blob/master/docs/syzbot.md#kmsan-bugs

Thank you. I will resend the patch later today with the proper tags.

Ciao
Stephan

[Eric Biggers]

Hi Stephan, it seems you never sent your patch out.

- Eric

[Stephan Müller]

Am Donnerstag, 5. Juli 2018, 01:37:57 CEST schrieb Eric Biggers:

Hi Eric,

Thank you for pointing this one out. At the time, I was searching for how I
can refer to the syzbot KMSAN branch that was used to produce the bug report.
I only see guidance on how to point to the Linux kernel tree.

Do you have a hint how to point to a different syzbot tree?

Ciao
Stephan

[Dmitry Vyukov]

Hi Stephan,

The general info about patch testing is here:

https://github.com/google/syzkaller/blob/master/docs/syzbot.md#testing-patches

Some additional KMSAN-specific info is at the bottom of the page:

https://github.com/google/syzkaller/blob/master/docs/syzbot.md#kmsan-bugs

In sort, you issue test command against
https://github.com/google/kmsan.git master and attach the patch.
The git tree/branch are also referenced in the syzbot report:
https://groups.google.com/forum/#!msg/syzkaller-bugs/nCuxVFfvc0I/zE1-hC3lCAAJ

Where did you see instructions mentioning Linus tree? I don't see we
ever refer to that tree in the instructions.

Thanks

[Stephan Müller]

Changes v2:
* Addition of syz testing line

---8<---

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 <smue...@chronox.de>
Reported-by: syzbot+9c251b...@syzkaller.appspotmail.com

#syz test: https://github.com/google/kmsan.git

---
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 49fa8582138b..bd6795ff406a 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -1148,8 +1148,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(&rsgl->sgl, &msg->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.17.1

[syzbot]

Hello,

syzbot tried to test the proposed patch but build/boot failed:

failed to checkout kernel repo https://github.com/google/kmsan.git/---:
failed to run /usr/bin/git [git fetch https://github.com/google/kmsan.git
---]: exit status 129
error: unknown option `-'
usage: git fetch [<options>] [<repository> [<refspec>...]]
or: git fetch [<options>] <group>
or: git fetch --multiple [<options>] [(<repository> | <group>)...]
or: git fetch --all [<options>]

-v, --verbose be more verbose
-q, --quiet be more quiet
--all fetch from all remotes
-a, --append append to .git/FETCH_HEAD instead of overwriting
--upload-pack <path> path to upload pack on remote end
-f, --force force overwrite of local branch
-m, --multiple fetch from multiple remotes
-t, --tags fetch all tags and associated objects
-n do not fetch all tags (--no-tags)
-p, --prune prune remote-tracking branches no longer on remote
--recurse-submodules[=<on-demand>]
control recursive fetching of submodules
--dry-run dry run
-k, --keep keep downloaded pack
-u, --update-head-ok allow updating of HEAD ref
--progress force progress reporting
--depth <depth> deepen history of shallow clone
--unshallow convert to a complete repository
--update-shallow accept refs that update .git/shallow
--refmap <refmap> specify fetch refmap




Tested on:

commit: [unknown]
git tree: https://github.com/google/kmsan.git/---
compiler: clang version 7.0.0 (trunk 334104)
patch: https://syzkaller.appspot.com/x/patch.diff?x=1207511c400000

[Stephan Müller]

Am Donnerstag, 5. Juli 2018, 19:02:01 CEST schrieb syzbot:

Hi Dimitry,

does the syzkaller somehow uses the "---" separator as part of the URL?

Thanks

Ciao
Stephan

[Dmitry Vyukov]

On Thu, Jul 5, 2018 at 8:45 PM, Stephan Müller <smue...@chronox.de> wrote:
> Am Donnerstag, 5. Juli 2018, 19:02:01 CEST schrieb syzbot:
>
> Hi Dimitry,
>
> does the syzkaller somehow uses the "---" separator as part of the URL?

It used it as branch. Please see:

https://github.com/google/syzkaller/blob/master/docs/syzbot.md#testing-patches

for formats. In all formats a git tree is not enough. And it is not
enough to identify code state in any other context too, it's always
git repo + branch or commit hash.

> --
> 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/1626520.Rx0128ICKU%40positron.chronox.de.

[Stephan Mueller]

Am Freitag, 6. Juli 2018, 09:38:41 CEST schrieb Dmitry Vyukov:

Hi Dmitry,

> On Thu, Jul 5, 2018 at 8:45 PM, Stephan Müller <smue...@chronox.de> wrote:
> > Am Donnerstag, 5. Juli 2018, 19:02:01 CEST schrieb syzbot:
> >
> > Hi Dimitry,
> >
> > does the syzkaller somehow uses the "---" separator as part of the URL?
>
> It used it as branch. Please see:
>
> https://github.com/google/syzkaller/blob/master/docs/syzbot.md#testing-patch
> es
>
> for formats. In all formats a git tree is not enough. And it is not
> enough to identify code state in any other context too, it's always
> git repo + branch or commit hash.

And which branch should I use for the kmsan.git repo?

Ciao
Stephan

[Dmitry Vyukov]

master, as specified in the original syzbot report. I will add this to
the doc too.

[Stephan Müller]

Changes v3:
* Fix syz testing line

Changes v2:
* Addition of syz testing line

---8<---

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 <smue...@chronox.de>
Reported-by: syzbot+9c251b...@syzkaller.appspotmail.com

#syz test: https://github.com/google/kmsan.git master

[Dmitry Vyukov]

On Fri, Jul 6, 2018 at 9:50 AM, Stephan Müller <smue...@chronox.de> wrote:
> Changes v3:
> * Fix syz testing line

Just in case, the syz test does not have to be in the patch. Just an
email to the syzbot address will do.

> --
> 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/1616306.R4SzcgHSdy%40positron.chronox.de.

[syzbot]

Hello,

syzbot tried to test the proposed patch but build/boot failed:

lost connection to test machine



[....] Starting enhanced syslogd: rsyslogd [?25l [?1c 7 [1G[ [32m ok
[39;49m 8 [?25h [?0c.
[....] Starting periodic command scheduler: cron [?25l [?1c 7 [1G[ [32m ok
[39;49m 8 [?25h [?0c.
[....] Starting OpenBSD Secure Shell server: sshd[ 21.709280] random:
sshd: uninitialized urandom read (32 bytes read)
[?25l [?1c 7 [1G[ [32m ok [39;49m 8 [?25h [?0c.

Debian GNU/Linux 7 syzkaller ttyS0

syzkaller login: [ 26.229113] random: sshd: uninitialized urandom read
(32 bytes read)
[ 26.532843] random: sshd: uninitialized urandom read (32 bytes read)
[ 27.787277] random: sshd: uninitialized urandom read (32 bytes read)
Warning: Permanently added '10.128.0.2' (ECDSA) to the list of known hosts.
[ 33.299368] random: sshd: uninitialized urandom read (32 bytes read)
flag provided but not defined: -os
Usage of ./syz-fuzzer:
-abort_signal int
initial signal to send to executor in error conditions; upgrades to
SIGKILL if executor does not exit
-arch string
target arch (default "amd64")
-buffer_size uint
internal buffer size (in bytes) for executor output
-collide
collide syscalls to provoke data races (default true)
-cover
collect feedback signals (coverage)
-debug
debug output from executor
-executor string
path to executor binary (default "./syz-executor")
-ipc string
ipc scheme (pipe/shmem)
-leak
detect memory leaks
-manager string
manager rpc address
-name string
unique name for manager (default "test")
-output string
write programs to none/stdout/dmesg/file (default "stdout")
-pprof string
address to serve pprof profiles
-procs int
number of parallel test processes (default 1)
-sandbox string
sandbox for fuzzing (none/setuid/namespace) (default "none")
-test
enable image testing mode
-threaded
use threaded mode in executor (default true)
-timeout duration
execution timeout
-v int
verbosity



Tested on:

commit: 9c9df9f275f0 kmsan: remove kmsan_threads_ready
git tree: https://github.com/google/kmsan.git/master
kernel config: https://syzkaller.appspot.com/x/.config?x=b11f4cfb262ee607

compiler: clang version 7.0.0 (trunk 334104)

patch: https://syzkaller.appspot.com/x/patch.diff?x=16a5af84400000

[Dmitry Vyukov]

On Fri, Jul 6, 2018 at 10:09 AM, syzbot
<syzbot+9c251b...@syzkaller.appspotmail.com> wrote:
> Hello,
>
> syzbot tried to test the proposed patch but build/boot failed:
>
> lost connection to test machine

Looking into this.

> --
> 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/000000000000363e2e0570502d42%40google.com.

[Dmitry Vyukov]

On Fri, Jul 6, 2018 at 10:19 AM, Dmitry Vyukov <dvy...@google.com> wrote:
> On Fri, Jul 6, 2018 at 10:09 AM, syzbot
> <syzbot+9c251b...@syzkaller.appspotmail.com> wrote:
>> Hello,
>>
>> syzbot tried to test the proposed patch but build/boot failed:
>>
>> lost connection to test machine
>
> Looking into this.

Should be fixed now, let's try again:

#syz test: https://github.com/google/kmsan.git master

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger
crash:

Reported-and-tested-by:
syzbot+9c251b...@syzkaller.appspotmail.com

Tested on:

commit: a00de5aa4da3 kmsan: delete some dead code

git tree: https://github.com/google/kmsan.git/master
kernel config: https://syzkaller.appspot.com/x/.config?x=b11f4cfb262ee607
compiler: clang version 7.0.0 (trunk 334104)

patch: https://syzkaller.appspot.com/x/patch.diff?x=13194968400000

Note: testing is done by a robot and is best-effort only.

[Stephan Mueller]

Am Freitag, 6. Juli 2018, 10:19:07 CEST schrieb Dmitry Vyukov:

Hi Dmitry,

> On Fri, Jul 6, 2018 at 10:09 AM, syzbot
>
> <syzbot+9c251b...@syzkaller.appspotmail.com> wrote:
> > Hello,
> >
> > syzbot tried to test the proposed patch but build/boot failed:
> >
> > lost connection to test machine
>
> Looking into this.

syzkaller reported the following which implies that the patch seems to fix the
issue.

syzbot has tested the proposed patch and the reproducer did not trigger
crash:

Reported-and-tested-by:
syzbot+9c251b...@syzkaller.appspotmail.com

Tested on:

commit: a00de5aa4da3 kmsan: delete some dead code

git tree: https://github.com/google/kmsan.git/master
kernel config: https://syzkaller.appspot.com/x/.config?x=b11f4cfb262ee607
compiler: clang version 7.0.0 (trunk 334104)

patch: https://syzkaller.appspot.com/x/patch.diff?x=13194968400000

Note: testing is done by a robot and is best-effort only.

Ciao
Stephan

[Eric Biggers]

On Fri, Jul 06, 2018 at 09:50:55AM +0200, Stephan Müller wrote:
> Changes v3:
> * Fix syz testing line
>
> Changes v2:
> * Addition of syz testing line
>
> ---8<---
>
> 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 <smue...@chronox.de>
> Reported-by: syzbot+9c251b...@syzkaller.appspotmail.com
> #syz test: https://github.com/google/kmsan.git master

Can you add Fixes: and Cc: stable?

- Eric

[Stephan Müller]

Changes v4:
* Add Fixes and CC line

Changes v3:
* Fix syz testing line

Changes v2:
* Addition of syz testing line

---8<---

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 <smue...@chronox.de>
Reported-by: syzbot+9c251b...@syzkaller.appspotmail.com
#syz test: https://github.com/google/kmsan.git master

CC: <sta...@vger.kernel.org> #4.14
Fixes: e870456d8e7c ("crypto: algif_skcipher - overhaul memory management")
Fixes: d887c52d6ae4 ("crypto: algif_aead - overhaul memory management")

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger
crash:

Reported-and-tested-by:
syzbot+9c251b...@syzkaller.appspotmail.com

Tested on:

commit: a00de5aa4da3 kmsan: delete some dead code
git tree: https://github.com/google/kmsan.git/master
kernel config: https://syzkaller.appspot.com/x/.config?x=b11f4cfb262ee607
compiler: clang version 7.0.0 (trunk 334104)

patch: https://syzkaller.appspot.com/x/patch.diff?x=17a9badc400000

[Herbert Xu]

On Sat, Jul 07, 2018 at 08:41:47PM +0200, Stephan Müller wrote:
> Changes v4:
> * Add Fixes and CC line
>
> Changes v3:
> * Fix syz testing line
>
> Changes v2:
> * Addition of syz testing line
>
> ---8<---
>
> 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 <smue...@chronox.de>
> Reported-by: syzbot+9c251b...@syzkaller.appspotmail.com
> #syz test: https://github.com/google/kmsan.git master
> CC: <sta...@vger.kernel.org> #4.14
> Fixes: e870456d8e7c ("crypto: algif_skcipher - overhaul memory management")
> Fixes: d887c52d6ae4 ("crypto: algif_aead - overhaul memory management")

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