KMSAN: uninit-value in af_alg_free_areq_sgls

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+9c251bdd09f83b92ba95@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 syzkaller@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.

[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 <smueller@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 <smueller@chronox.de>
Reported-by: syzbot+9c251bdd09f83b92ba95@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

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

[Dmitry Vyukov]

On Sun, Apr 8, 2018 at 7:57 PM, Stephan Müller <smueller@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




> The fix below should ensure this now.
>
> Thanks a lot.
>
> ---8<---
>
> From 8f083e7b0684a9f91c186d7b46eec34e439689c3 Mon Sep 17 00:00:00 2001
> From: Stephan Mueller <smueller@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 <smueller@chronox.de>
> Reported-by: syzbot+9c251bdd09f83b92ba95@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
>
>
>
>
>
> --
> 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+unsubscribe@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.

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

[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

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

[Eric Biggers]

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

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

[Stephan Müller]

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

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

[Dmitry Vyukov]

On Thu, Jul 5, 2018 at 9:49 AM, Stephan Müller <smueller@chronox.de> 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

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

[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 <smueller@chronox.de>
Reported-by: syzbot+9c251bdd09f83b92ba95@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

Re: KMSAN: uninit-value in af_alg_free_areq_sgls

[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

Re: KMSAN: uninit-value in af_alg_free_areq_sgls

[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

> 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

Re: KMSAN: uninit-value in af_alg_free_areq_sgls

[Dmitry Vyukov]

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


>> 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 syzkaller-bugs+unsubscribe@googlegroups.com.
> 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.

Re: KMSAN: uninit-value in af_alg_free_areq_sgls

[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 <smueller@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

Re: KMSAN: uninit-value in af_alg_free_areq_sgls

[Dmitry Vyukov]

On Fri, Jul 6, 2018 at 9:41 AM, Stephan Mueller <smueller@chronox.de> 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 <smueller@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?

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

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

[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 <smueller@chronox.de>
Reported-by: syzbot+9c251bdd09f83b92ba95@syzkaller.appspotmail.com
#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

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

[Dmitry Vyukov]

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


> 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 <smueller@chronox.de>
> Reported-by: syzbot+9c251bdd09f83b92ba95@syzkaller.appspotmail.com
> #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 syzkaller-bugs+unsubscribe@googlegroups.com.
> 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.

Re: KMSAN: uninit-value in af_alg_free_areq_sgls

[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

Re: KMSAN: uninit-value in af_alg_free_areq_sgls

[Dmitry Vyukov]

On Fri, Jul 6, 2018 at 10:09 AM, syzbot
<syzbot+9c251bdd09f83b92ba95@syzkaller.appspotmail.com> 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 syzkaller-bugs+unsubscribe@googlegroups.com.
> 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.

Re: KMSAN: uninit-value in af_alg_free_areq_sgls

[Dmitry Vyukov]

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

On Fri, Jul 6, 2018 at 10:19 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Fri, Jul 6, 2018 at 10:09 AM, syzbot
> <syzbot+9c251bdd09f83b92ba95@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


>> [....] 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 syzkaller-bugs+unsubscribe@googlegroups.com.
>> 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.

[-- Attachment #2: alg.patch --]
[-- Type: text/x-patch, Size: 409 bytes --]

--- 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)

Re: KMSAN: uninit-value in af_alg_free_areq_sgls

[syzbot]

Hello,

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

Reported-and-tested-by:  
syzbot+9c251bdd09f83b92ba95@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.

Re: KMSAN: uninit-value in af_alg_free_areq_sgls

[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+9c251bdd09f83b92ba95@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+9c251bdd09f83b92ba95@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

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

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

Can you add Fixes: and Cc: stable?

- Eric

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

[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 <smueller@chronox.de>
Reported-by: syzbot+9c251bdd09f83b92ba95@syzkaller.appspotmail.com
#syz test: https://github.com/google/kmsan.git master
CC: <stable@vger.kernel.org> #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

Re: KMSAN: uninit-value in af_alg_free_areq_sgls

[syzbot]

Hello,

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

Reported-and-tested-by:  
syzbot+9c251bdd09f83b92ba95@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

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

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

[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 <smueller@chronox.de>
> Reported-by: syzbot+9c251bdd09f83b92ba95@syzkaller.appspotmail.com
> #syz test: https://github.com/google/kmsan.git master
> CC: <stable@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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt