2018-04-08 07:07:01

by syzbot

[permalink] [raw]
Subject: KMSAN: uninit-value in af_alg_free_areq_sgls

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: [email protected]
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 [email protected].

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.


2018-04-08 17:57:12

by Stephan Müller

[permalink] [raw]
Subject: [PATCH] AF_ALG: register completely initialized request in list

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 <[email protected]>
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 <[email protected]>
Reported-by: [email protected]
---
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

2018-04-09 07:51:13

by Dmitry Vyukov

[permalink] [raw]
Subject: Re: [PATCH] AF_ALG: register completely initialized request in list

On Sun, Apr 8, 2018 at 7:57 PM, Stephan Müller <[email protected]> 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




> The fix below should ensure this now.
>
> Thanks a lot.
>
> ---8<---
>
> From 8f083e7b0684a9f91c186d7b46eec34e439689c3 Mon Sep 17 00:00:00 2001
> From: Stephan Mueller <[email protected]>
> 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 <[email protected]>
> Reported-by: [email protected]
> ---
> 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
>
>
>
>
>
> --
> 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 [email protected].
> 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.

2018-04-09 07:54:12

by Stephan Müller

[permalink] [raw]
Subject: Re: [PATCH] AF_ALG: register completely initialized request in list

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

2018-07-04 23:37:57

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH] AF_ALG: register completely initialized request in list

On Mon, Apr 09, 2018 at 09:54:12AM +0200, Stephan Mueller wrote:
> 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

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

- Eric

2018-07-05 07:49:41

by Stephan Müller

[permalink] [raw]
Subject: Re: [PATCH] AF_ALG: register completely initialized request in list

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

Hi Eric,

> On Mon, Apr 09, 2018 at 09:54:12AM +0200, Stephan Mueller wrote:
> > 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#communica
> > > tion -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-bug
> > > s
> >
> > Thank you. I will resend the patch later today with the proper tags.
> >
> > Ciao
> > Stephan
>
> Hi Stephan, it seems you never sent your patch out.

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

2018-07-05 08:43:10

by Dmitry Vyukov

[permalink] [raw]
Subject: Re: [PATCH] AF_ALG: register completely initialized request in list

On Thu, Jul 5, 2018 at 9:49 AM, Stephan Müller <[email protected]> wrote:
> Am Donnerstag, 5. Juli 2018, 01:37:57 CEST schrieb Eric Biggers:
>
> Hi Eric,
>
>> On Mon, Apr 09, 2018 at 09:54:12AM +0200, Stephan Mueller wrote:
>> > 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#communica
>> > > tion -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-bug
>> > > s
>> >
>> > Thank you. I will resend the patch later today with the proper tags.
>> >
>> > Ciao
>> > Stephan
>>
>> Hi Stephan, it seems you never sent your patch out.
>
> 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?

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

2018-07-05 15:58:15

by Stephan Müller

[permalink] [raw]
Subject: [PATCH v2] AF_ALG: Initialize sg_num_bytes in error code path

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 <[email protected]>
Reported-by: [email protected]
#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

2018-07-05 17:02:01

by syzbot

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in af_alg_free_areq_sgls

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

2018-07-05 18:45:23

by Stephan Müller

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in af_alg_free_areq_sgls

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

> 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


Ciao
Stephan

2018-07-06 07:38:41

by Dmitry Vyukov

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in af_alg_free_areq_sgls

On Thu, Jul 5, 2018 at 8:45 PM, Stephan Müller <[email protected]> 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.


>> 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
>
>
> 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 [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/1626520.Rx0128ICKU%40positron.chronox.de.
> For more options, visit https://groups.google.com/d/optout.

2018-07-06 07:41:53

by Stephan Müller

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in af_alg_free_areq_sgls

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 <[email protected]> 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

2018-07-06 07:44:23

by Dmitry Vyukov

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in af_alg_free_areq_sgls

On Fri, Jul 6, 2018 at 9:41 AM, Stephan Mueller <[email protected]> wrote:
> 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 <[email protected]> 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?

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

2018-07-06 07:50:55

by Stephan Müller

[permalink] [raw]
Subject: [PATCH v3] AF_ALG: Initialize sg_num_bytes in error code path

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 <[email protected]>
Reported-by: [email protected]
#syz test: https://github.com/google/kmsan.git master
---
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

2018-07-06 07:58:10

by Dmitry Vyukov

[permalink] [raw]
Subject: Re: [PATCH v3] AF_ALG: Initialize sg_num_bytes in error code path

On Fri, Jul 6, 2018 at 9:50 AM, Stephan Müller <[email protected]> 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.


> 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 <[email protected]>
> Reported-by: [email protected]
> #syz test: https://github.com/google/kmsan.git master
> ---
> 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
>
>
>
>
> --
> 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 [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/1616306.R4SzcgHSdy%40positron.chronox.de.
> For more options, visit https://groups.google.com/d/optout.

2018-07-06 08:09:01

by syzbot

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in af_alg_free_areq_sgls

Hello,

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

lost connection to test machine



[....] Starting enhanced syslogd: rsyslogd[?25l[?1c7[ ok
8[?25h[?0c.
[....] Starting periodic command scheduler: cron[?25l[?1c7[ ok
8[?25h[?0c.
[....] Starting OpenBSD Secure Shell server: sshd[ 21.709280] random:
sshd: uninitialized urandom read (32 bytes read)
[?25l[?1c7[ ok 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

2018-07-06 08:19:07

by Dmitry Vyukov

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in af_alg_free_areq_sgls

On Fri, Jul 6, 2018 at 10:09 AM, syzbot
<[email protected]> wrote:
> Hello,
>
> syzbot tried to test the proposed patch but build/boot failed:
>
> lost connection to test machine

Looking into this.

> [....] 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
>
> --
> 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 [email protected].
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/syzkaller-bugs/000000000000363e2e0570502d42%40google.com.
>
> For more options, visit https://groups.google.com/d/optout.

2018-07-06 16:27:07

by Stephan Müller

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in af_alg_free_areq_sgls

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

Hi Dmitry,

> On Fri, Jul 6, 2018 at 10:09 AM, syzbot
>
> <[email protected]> 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:
[email protected]

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

2018-07-06 21:57:51

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH v3] AF_ALG: Initialize sg_num_bytes in error code path

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 <[email protected]>
> Reported-by: [email protected]
> #syz test: https://github.com/google/kmsan.git master

Can you add Fixes: and Cc: stable?

- Eric

2018-07-07 18:41:47

by Stephan Müller

[permalink] [raw]
Subject: [PATCH v4] AF_ALG: Initialize sg_num_bytes in error code path

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 <[email protected]>
Reported-by: [email protected]
#syz test: https://github.com/google/kmsan.git master
CC: <[email protected]> #4.14
Fixes: e870456d8e7c ("crypto: algif_skcipher - overhaul memory management")
Fixes: d887c52d6ae4 ("crypto: algif_aead - overhaul memory management")
---
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

2018-07-07 19:01:02

by syzbot

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in af_alg_free_areq_sgls

Hello,

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

Reported-and-tested-by:
[email protected]

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

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

2018-07-13 10:34:10

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v4] AF_ALG: Initialize sg_num_bytes in error code path

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 <[email protected]>
> Reported-by: [email protected]
> #syz test: https://github.com/google/kmsan.git master
> CC: <[email protected]> #4.14
> Fixes: e870456d8e7c ("crypto: algif_skcipher - overhaul memory management")
> Fixes: d887c52d6ae4 ("crypto: algif_aead - overhaul memory management")

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