[syzbot] [crypto?] KCSAN: data-race in random_recv_done / virtio_read (3)

11 views
Skip to first unread message

syzbot

unread,
Apr 21, 2023, 10:35:59 AM4/21/23
to da...@davemloft.net, her...@gondor.apana.org.au, linux-...@vger.kernel.org, linux-...@vger.kernel.org, oli...@selenic.com, syzkall...@googlegroups.com
Hello,

syzbot found the following issue on:

HEAD commit: 2faac9a98f01 Merge tag 'keys-fixes-20230321' of git://git...
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1113f21cc80000
kernel config: https://syzkaller.appspot.com/x/.config?x=3eb0bb0ae89a5345
dashboard link: https://syzkaller.appspot.com/bug?extid=726dc8c62c3536431ceb
compiler: Debian clang version 15.0.7, GNU ld (GNU Binutils for Debian) 2.35.2

Unfortunately, I don't have any reproducer for this issue yet.

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/571c9c5a3db2/disk-2faac9a9.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/a051e3d7c495/vmlinux-2faac9a9.xz
kernel image: https://storage.googleapis.com/syzbot-assets/ff5ec0d6e37d/bzImage-2faac9a9.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+726dc8...@syzkaller.appspotmail.com

==================================================================
BUG: KCSAN: data-race in random_recv_done / virtio_read

read to 0xffff8881019054ec of 4 bytes by task 14079 on cpu 0:
copy_data drivers/char/hw_random/virtio-rng.c:70 [inline]
virtio_read+0xc3/0x3f0 drivers/char/hw_random/virtio-rng.c:92
rng_get_data drivers/char/hw_random/core.c:197 [inline]
rng_dev_read+0x1a7/0x5e0 drivers/char/hw_random/core.c:234
vfs_read+0x192/0x560 fs/read_write.c:468
ksys_read+0xeb/0x1a0 fs/read_write.c:613
__do_sys_read fs/read_write.c:623 [inline]
__se_sys_read fs/read_write.c:621 [inline]
__x64_sys_read+0x42/0x50 fs/read_write.c:621
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd

write to 0xffff8881019054ec of 4 bytes by interrupt on cpu 1:
random_recv_done+0x62/0x90 drivers/char/hw_random/virtio-rng.c:45
vring_interrupt+0x150/0x170 drivers/virtio/virtio_ring.c:2491
__handle_irq_event_percpu+0x91/0x490 kernel/irq/handle.c:158
handle_irq_event_percpu kernel/irq/handle.c:193 [inline]
handle_irq_event+0x64/0xf0 kernel/irq/handle.c:210
handle_edge_irq+0x17f/0x5a0 kernel/irq/chip.c:819
generic_handle_irq_desc include/linux/irqdesc.h:158 [inline]
handle_irq arch/x86/kernel/irq.c:231 [inline]
__common_interrupt+0x64/0x100 arch/x86/kernel/irq.c:250
common_interrupt+0x49/0xc0 arch/x86/kernel/irq.c:240
asm_common_interrupt+0x26/0x40 arch/x86/include/asm/idtentry.h:636

value changed: 0x00000000 -> 0x00000040

Reported by Kernel Concurrency Sanitizer on:
CPU: 1 PID: 14077 Comm: syz-executor.2 Not tainted 6.3.0-rc3-syzkaller-00016-g2faac9a98f01 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/02/2023
==================================================================
==================================================================
BUG: KCSAN: data-race in detach_buf_split / virtqueue_add

read to 0xffff888101a76950 of 4 bytes by task 14131 on cpu 0:
virtqueue_add_split drivers/virtio/virtio_ring.c:553 [inline]
virtqueue_add+0x4b9/0x2130 drivers/virtio/virtio_ring.c:2117
virtqueue_add_inbuf+0x53/0x80 drivers/virtio/virtio_ring.c:2196
request_entropy drivers/char/hw_random/virtio-rng.c:61 [inline]
copy_data drivers/char/hw_random/virtio-rng.c:74 [inline]
virtio_read+0x1c5/0x3f0 drivers/char/hw_random/virtio-rng.c:92
rng_get_data drivers/char/hw_random/core.c:197 [inline]
rng_dev_read+0x1a7/0x5e0 drivers/char/hw_random/core.c:234
vfs_read+0x192/0x560 fs/read_write.c:468
ksys_read+0xeb/0x1a0 fs/read_write.c:613
__do_sys_read fs/read_write.c:623 [inline]
__se_sys_read fs/read_write.c:621 [inline]
__x64_sys_read+0x42/0x50 fs/read_write.c:621
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd

read-write to 0xffff888101a76950 of 4 bytes by interrupt on cpu 1:
detach_buf_split+0x2fc/0x570 drivers/virtio/virtio_ring.c:757
virtqueue_get_buf_ctx_split drivers/virtio/virtio_ring.c:835 [inline]
virtqueue_get_buf_ctx+0x3c8/0x5c0 drivers/virtio/virtio_ring.c:2311
virtqueue_get_buf+0x1f/0x30 drivers/virtio/virtio_ring.c:2317
random_recv_done+0x4c/0x90 drivers/char/hw_random/virtio-rng.c:42
vring_interrupt+0x150/0x170 drivers/virtio/virtio_ring.c:2491
__handle_irq_event_percpu+0x91/0x490 kernel/irq/handle.c:158
handle_irq_event_percpu kernel/irq/handle.c:193 [inline]
handle_irq_event+0x64/0xf0 kernel/irq/handle.c:210
handle_edge_irq+0x17f/0x5a0 kernel/irq/chip.c:819
generic_handle_irq_desc include/linux/irqdesc.h:158 [inline]
handle_irq arch/x86/kernel/irq.c:231 [inline]
__common_interrupt+0x64/0x100 arch/x86/kernel/irq.c:250
common_interrupt+0x9e/0xc0 arch/x86/kernel/irq.c:240
asm_common_interrupt+0x26/0x40 arch/x86/include/asm/idtentry.h:636
xas_find+0x10a/0x3f0
find_get_entry mm/filemap.c:2008 [inline]
filemap_get_folios+0xa4/0x3f0 mm/filemap.c:2174
mpage_map_and_submit_buffers fs/ext4/inode.c:2358 [inline]
mpage_map_and_submit_extent fs/ext4/inode.c:2513 [inline]
ext4_do_writepages+0x1017/0x2140 fs/ext4/inode.c:2876
ext4_writepages+0x127/0x250 fs/ext4/inode.c:2964
do_writepages+0x1c5/0x340 mm/page-writeback.c:2551
filemap_fdatawrite_wbc+0xdb/0xf0 mm/filemap.c:390
__filemap_fdatawrite_range mm/filemap.c:423 [inline]
__filemap_fdatawrite mm/filemap.c:429 [inline]
filemap_flush+0x95/0xc0 mm/filemap.c:456
ext4_alloc_da_blocks+0x50/0x130 fs/ext4/inode.c:3218
ext4_release_file+0x5f/0x1c0 fs/ext4/file.c:158
__fput+0x245/0x570 fs/file_table.c:321
____fput+0x15/0x20 fs/file_table.c:349
task_work_run+0x123/0x160 kernel/task_work.c:179
exit_task_work include/linux/task_work.h:38 [inline]
do_exit+0x600/0x1710 kernel/exit.c:869
do_group_exit+0x101/0x150 kernel/exit.c:1019
get_signal+0xea9/0xfe0 kernel/signal.c:2859
arch_do_signal_or_restart+0x89/0x2b0 arch/x86/kernel/signal.c:306
exit_to_user_mode_loop+0x6d/0xe0 kernel/entry/common.c:168
exit_to_user_mode_prepare+0x6a/0xa0 kernel/entry/common.c:203
irqentry_exit_to_user_mode+0x9/0x20 kernel/entry/common.c:309
irqentry_exit+0x12/0x40 kernel/entry/common.c:412
exc_general_protection+0x339/0x4c0 arch/x86/kernel/traps.c:728
asm_exc_general_protection+0x26/0x30 arch/x86/include/asm/idtentry.h:564

value changed: 0x00000001 -> 0x00000000

Reported by Kernel Concurrency Sanitizer on:
CPU: 1 PID: 14098 Comm: syz-executor.1 Not tainted 6.3.0-rc3-syzkaller-00016-g2faac9a98f01 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/02/2023
==================================================================


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzk...@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

Dmitry Vyukov

unread,
Apr 21, 2023, 10:52:28 AM4/21/23
to syzbot, da...@davemloft.net, her...@gondor.apana.org.au, linux-...@vger.kernel.org, linux-...@vger.kernel.org, oli...@selenic.com, syzkall...@googlegroups.com
On Fri, 21 Apr 2023 at 16:36, syzbot
<syzbot+726dc8...@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit: 2faac9a98f01 Merge tag 'keys-fixes-20230321' of git://git...
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=1113f21cc80000
> kernel config: https://syzkaller.appspot.com/x/.config?x=3eb0bb0ae89a5345
> dashboard link: https://syzkaller.appspot.com/bug?extid=726dc8c62c3536431ceb
> compiler: Debian clang version 15.0.7, GNU ld (GNU Binutils for Debian) 2.35.2
>
> Unfortunately, I don't have any reproducer for this issue yet.
>
> Downloadable assets:
> disk image: https://storage.googleapis.com/syzbot-assets/571c9c5a3db2/disk-2faac9a9.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/a051e3d7c495/vmlinux-2faac9a9.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/ff5ec0d6e37d/bzImage-2faac9a9.xz
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+726dc8...@syzkaller.appspotmail.com

Here this:

size = min_t(unsigned int, size, vi->data_avail);
memcpy(buf, vi->data + vi->data_idx, size);
vi->data_idx += size;
vi->data_avail -= size;

runs concurrently with:

if (!virtqueue_get_buf(vi->vq, &vi->data_avail))
return;
vi->data_idx = 0;

I did not fully grasp how/where vi->data is populated, but it looks
like it can lead to use of uninit/stale random data, or even to out of
bounds access, say if vi->data_avail is already updated, but
vi->data_idx is not yet reset to 0. Then concurrent reading will read
not where it's supposed to read.

Herbert Xu

unread,
May 3, 2023, 6:46:07 AM5/3/23
to Dmitry Vyukov, syzbot, da...@davemloft.net, linux-...@vger.kernel.org, linux-...@vger.kernel.org, oli...@selenic.com, syzkall...@googlegroups.com, Jason Wang, Michael S. Tsirkin, Laurent Vivier
On Fri, Apr 21, 2023 at 04:52:13PM +0200, Dmitry Vyukov wrote:
>
> Here this:
>
> size = min_t(unsigned int, size, vi->data_avail);
> memcpy(buf, vi->data + vi->data_idx, size);
> vi->data_idx += size;
> vi->data_avail -= size;
>
> runs concurrently with:
>
> if (!virtqueue_get_buf(vi->vq, &vi->data_avail))
> return;
> vi->data_idx = 0;
>
> I did not fully grasp how/where vi->data is populated, but it looks
> like it can lead to use of uninit/stale random data, or even to out of
> bounds access, say if vi->data_avail is already updated, but
> vi->data_idx is not yet reset to 0. Then concurrent reading will read
> not where it's supposed to read.

This appears to be a genuine bug. I'll look into it.

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

Herbert Xu

unread,
May 3, 2023, 6:55:08 AM5/3/23
to Dmitry Vyukov, syzbot, da...@davemloft.net, linux-...@vger.kernel.org, linux-...@vger.kernel.org, oli...@selenic.com, syzkall...@googlegroups.com, Jason Wang, Michael S. Tsirkin, Laurent Vivier, Rusty Russell
On Fri, Apr 21, 2023 at 04:52:13PM +0200, Dmitry Vyukov wrote:
>
> Here this:
>
> size = min_t(unsigned int, size, vi->data_avail);
> memcpy(buf, vi->data + vi->data_idx, size);
> vi->data_idx += size;
> vi->data_avail -= size;
>
> runs concurrently with:
>
> if (!virtqueue_get_buf(vi->vq, &vi->data_avail))
> return;
> vi->data_idx = 0;
>
> I did not fully grasp how/where vi->data is populated, but it looks
> like it can lead to use of uninit/stale random data, or even to out of
> bounds access, say if vi->data_avail is already updated, but
> vi->data_idx is not yet reset to 0. Then concurrent reading will read
> not where it's supposed to read.

Yes this is a real race. This bug appears to have been around
forever.

---8<---
The virtio rng device kicks off a new entropy request whenever the
data available reaches zero. When a new request occurs at the end
of a read operation, that is, when the result of that request is
only needed by the next reader, then there is a race between the
writing of the new data and the next reader.

This is because there is no synchronisation whatsoever between the
writer and the reader.

Fix this by writing data_avail with smp_store_release and reading
it with smp_load_acquire when we first enter read. The subsequent
reads are safe because they're either protected by the first load
acquire, or by the completion mechanism.

Reported-by: syzbot+726dc8...@syzkaller.appspotmail.com
Fixes: f7f510ec1957 ("virtio: An entropy device, as suggested by hpa.")
Signed-off-by: Herbert Xu <her...@gondor.apana.org.au>

diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c
index f7690e0f92ed..e41a84e6b4b5 100644
--- a/drivers/char/hw_random/virtio-rng.c
+++ b/drivers/char/hw_random/virtio-rng.c
@@ -4,6 +4,7 @@
* Copyright (C) 2007, 2008 Rusty Russell IBM Corporation
*/

+#include <asm/barrier.h>
#include <linux/err.h>
#include <linux/hw_random.h>
#include <linux/scatterlist.h>
@@ -37,13 +38,13 @@ struct virtrng_info {
static void random_recv_done(struct virtqueue *vq)
{
struct virtrng_info *vi = vq->vdev->priv;
+ unsigned int len;

/* We can get spurious callbacks, e.g. shared IRQs + virtio_pci. */
- if (!virtqueue_get_buf(vi->vq, &vi->data_avail))
+ if (!virtqueue_get_buf(vi->vq, &len))
return;

- vi->data_idx = 0;
-
+ smp_store_release(&vi->data_avail, len);
complete(&vi->have_data);
}

@@ -52,7 +53,6 @@ static void request_entropy(struct virtrng_info *vi)
struct scatterlist sg;

reinit_completion(&vi->have_data);
- vi->data_avail = 0;
vi->data_idx = 0;

sg_init_one(&sg, vi->data, sizeof(vi->data));
@@ -88,7 +88,7 @@ static int virtio_read(struct hwrng *rng, void *buf, size_t size, bool wait)
read = 0;

/* copy available data */
- if (vi->data_avail) {
+ if (smp_load_acquire(&vi->data_avail)) {
chunk = copy_data(vi, buf, size);
size -= chunk;
read += chunk;

Tudor Ambarus

unread,
May 3, 2023, 7:19:34 AM5/3/23
to Herbert Xu, Dmitry Vyukov, syzbot, da...@davemloft.net, linux-...@vger.kernel.org, linux-...@vger.kernel.org, oli...@selenic.com, syzkall...@googlegroups.com, Jason Wang, Michael S. Tsirkin, Laurent Vivier, Rusty Russell
Hi,
Link: https://syzkaller.appspot.com/bug?extid=726dc8c62c3536431ceb

Please add the dashboard link if applying as searching for the syzbot ID
rarely gives meaningful results.

Cheers,
ta

Michael S. Tsirkin

unread,
May 3, 2023, 7:37:10 AM5/3/23
to Herbert Xu, Dmitry Vyukov, syzbot, da...@davemloft.net, linux-...@vger.kernel.org, linux-...@vger.kernel.org, oli...@selenic.com, syzkall...@googlegroups.com, Jason Wang, Laurent Vivier, Rusty Russell
On the surface of it, it looks like you removed this store
which isn't described in the commit log.
I do not, offhand, remember why we stored 0 in data_idx here
when we also zero it in request_entropy.
It was added with


commit 5c8e933050044d6dd2a000f9a5756ae73cbe7c44
Author: Laurent Vivier <lvi...@redhat.com>
Date: Thu Oct 28 12:11:10 2021 +0200

hwrng: virtio - don't waste entropy

if we don't use all the entropy available in the buffer, keep it
and use it later.

Signed-off-by: Laurent Vivier <lvi...@redhat.com>
Link: https://lore.kernel.org/r/20211028101111....@redhat.com
Signed-off-by: Michael S. Tsirkin <m...@redhat.com>

Herbert Xu

unread,
May 3, 2023, 11:56:14 PM5/3/23
to Tudor Ambarus, Dmitry Vyukov, syzbot, da...@davemloft.net, linux-...@vger.kernel.org, linux-...@vger.kernel.org, oli...@selenic.com, syzkall...@googlegroups.com, Jason Wang, Michael S. Tsirkin, Laurent Vivier, Rusty Russell
On Wed, May 03, 2023 at 12:19:30PM +0100, Tudor Ambarus wrote:
>
> > Reported-by: syzbot+726dc8...@syzkaller.appspotmail.com
>
> Link: https://syzkaller.appspot.com/bug?extid=726dc8c62c3536431ceb
>
> Please add the dashboard link if applying as searching for the syzbot ID
> rarely gives meaningful results.

The syzbot ID is already present in the in the Reported-by tag.
There is no reason to clutter up the commit message with redundant
information.

Cheers,

Herbert Xu

unread,
May 3, 2023, 11:59:52 PM5/3/23
to Michael S. Tsirkin, Dmitry Vyukov, syzbot, da...@davemloft.net, linux-...@vger.kernel.org, linux-...@vger.kernel.org, oli...@selenic.com, syzkall...@googlegroups.com, Jason Wang, Laurent Vivier, Rusty Russell
On Wed, May 03, 2023 at 07:37:00AM -0400, Michael S. Tsirkin wrote:
>
> On the surface of it, it looks like you removed this store
> which isn't described in the commit log.
> I do not, offhand, remember why we stored 0 in data_idx here
> when we also zero it in request_entropy.
> It was added with

Yes I removed because it's redundant. But you're right I'll add
a note about it in the log:

---8<---
The virtio rng device kicks off a new entropy request whenever the
data available reaches zero. When a new request occurs at the end
of a read operation, that is, when the result of that request is
only needed by the next reader, then there is a race between the
writing of the new data and the next reader.

This is because there is no synchronisation whatsoever between the
writer and the reader.

Fix this by writing data_avail with smp_store_release and reading
it with smp_load_acquire when we first enter read. The subsequent
reads are safe because they're either protected by the first load
acquire, or by the completion mechanism.

Also remove the redundant zeroing of data_idx in random_recv_done
(data_idx must already be zero at this point) and data_avail in
request_entropy (ditto).

Reported-by: syzbot+726dc8...@syzkaller.appspotmail.com
Fixes: f7f510ec1957 ("virtio: An entropy device, as suggested by hpa.")
Signed-off-by: Herbert Xu <her...@gondor.apana.org.au>

diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c
index f7690e0f92ed..e41a84e6b4b5 100644
--- a/drivers/char/hw_random/virtio-rng.c
+++ b/drivers/char/hw_random/virtio-rng.c
@@ -4,6 +4,7 @@
* Copyright (C) 2007, 2008 Rusty Russell IBM Corporation
*/

+#include <asm/barrier.h>
#include <linux/err.h>
#include <linux/hw_random.h>
#include <linux/scatterlist.h>
@@ -37,13 +38,13 @@ struct virtrng_info {
static void random_recv_done(struct virtqueue *vq)
{
struct virtrng_info *vi = vq->vdev->priv;
+ unsigned int len;

/* We can get spurious callbacks, e.g. shared IRQs + virtio_pci. */
- if (!virtqueue_get_buf(vi->vq, &vi->data_avail))
+ if (!virtqueue_get_buf(vi->vq, &len))
return;

- vi->data_idx = 0;
-

Michael S. Tsirkin

unread,
May 4, 2023, 1:28:39 AM5/4/23
to Herbert Xu, Dmitry Vyukov, syzbot, da...@davemloft.net, linux-...@vger.kernel.org, linux-...@vger.kernel.org, oli...@selenic.com, syzkall...@googlegroups.com, Jason Wang, Laurent Vivier, Rusty Russell
Acked-by: Michael S. Tsirkin <m...@redhat.com>

feel free ro merge, thanks!

Tudor Ambarus

unread,
May 4, 2023, 4:10:47 AM5/4/23
to Herbert Xu, Dmitry Vyukov, syzbot, da...@davemloft.net, linux-...@vger.kernel.org, linux-...@vger.kernel.org, oli...@selenic.com, syzkall...@googlegroups.com, Jason Wang, Michael S. Tsirkin, Laurent Vivier, Rusty Russell


On 5/4/23 04:55, Herbert Xu wrote:
> On Wed, May 03, 2023 at 12:19:30PM +0100, Tudor Ambarus wrote:
>>
>>> Reported-by: syzbot+726dc8...@syzkaller.appspotmail.com
>>
>> Link: https://syzkaller.appspot.com/bug?extid=726dc8c62c3536431ceb
>>
>> Please add the dashboard link if applying as searching for the syzbot ID
>> rarely gives meaningful results.
>
> The syzbot ID is already present in the in the Reported-by tag.
> There is no reason to clutter up the commit message with redundant
> information.
>

As you prefer. Theodore Ts'o encourages to add a dashboard link, here's
his reasoning:
https://github.com/google/syzkaller/issues/3393#issuecomment-1347476434

Cheers,
ta

Theodore Ts'o

unread,
May 5, 2023, 12:01:49 AM5/5/23
to Tudor Ambarus, Herbert Xu, Dmitry Vyukov, syzbot, da...@davemloft.net, linux-...@vger.kernel.org, linux-...@vger.kernel.org, oli...@selenic.com, syzkall...@googlegroups.com, Jason Wang, Michael S. Tsirkin, Laurent Vivier, Rusty Russell, Aleksandr Nogikh
On Thu, May 04, 2023 at 09:10:43AM +0100, Tudor Ambarus wrote:
> > The syzbot ID is already present in the in the Reported-by tag.
> > There is no reason to clutter up the commit message with redundant
> > information.
>
> As you prefer. Theodore Ts'o encourages to add a dashboard link, here's
> his reasoning:
> https://github.com/google/syzkaller/issues/3393#issuecomment-1347476434

The reason why I've requested having both the Link and Reported-by is
because you don't know the secret incantation:

s;Reported-by: syzbot\+\([0-9a-z]+\)@syzkaller.appspotmail.com;https://syzkaller.appspotmail.com/extid?=\1;

... you can't easily get from a "Reported-by:" e-mail address to a URL
link that will actually get you to the syzkaller page. What I used to
do was to go to https://groups.google.com/g/syzkaller-bugs and then
enter into the Google Groups searech box:

Reported-by: syzbot+726dc8...@syzkaller.appspotmail.com

which is a ***super*** clunky way to get to the syzkaller page. What
would be nice is if there was an easy way that didn't rely on kernel
developers knowing the internal URL structure of Syzbot to be able to
enter the Reported-by link on some convenient web page, perhaps in a
search box found in the front page of https://syzkaller.appspot.com,
and be able to find the syzbot report web page that way.

Since that doesn't exist today, I include both the Reported-by: and
Link: in my commit descriptions, out of consideration to the reviewer
who might want to be able to find the Syzbot page and don't know the
secret trick to calculate the URL from the Reported-by: e-mail
address.


Another gotcha with Syzbot is that there are two id's, the "extid" and
the "id" which makes thing ***super*** confusing. For example, both
of these URL's go the same Syzbot report:

https://syzkaller.appspot.com/bug?extid=726dc8c62c3536431ceb
https://syzkaller.appspot.com/bug?id=eec08eb3763c9ec749fd565e70cfe6e485af7ed7

The Reported-by e-mail address uses the extid. So for example, this
case, it would be syzbot+726dc8...@syzkaller.appspotmail.com.

However, all of the links in the Syzbot web pages use the id form of
the URL. So if you were browsing the syzbot reports assigned to the
crypto subsystem via https://syzkaller.appspot.com/upstream/s/crypto,
you would find the id-style link, and then the commit fixing the bug
might have something like this:

Reported-by: syzbot+726dc8...@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?id=eec08eb3763c9ec749fd565e70cfe6e485af7ed7

In that case, there is no (obvious) relationship between the hex
string found in the Reported-by line and the Link line.


One additional unfortunate fallout from syzbot having an "extid" and
"id", is that depending on how the syzbot entry initially found by the
contributor sending in a patch to address a syzbot report, either URL
can be found in mailing list archives. So if you search for
"extid=726dc8c62c3536431ceb" you won't find references to
"id=eec08eb3763c9ec749fd565e70cfe6e485af7ed7" even though they are
both referring to same Syzbot report.

<<< sigh >>>> As they say, the hardest problem to solve in the
C.S. world is naming, and syzbot has two names for every single syzbot
report, and both are exposed to the poor user. :-(

- Ted

Dmitry Vyukov

unread,
May 8, 2023, 1:33:55 AM5/8/23
to Theodore Ts'o, Tudor Ambarus, Herbert Xu, syzbot, da...@davemloft.net, linux-...@vger.kernel.org, linux-...@vger.kernel.org, oli...@selenic.com, syzkall...@googlegroups.com, Jason Wang, Michael S. Tsirkin, Laurent Vivier, Rusty Russell, Aleksandr Nogikh, syzkaller
A link like this may work for syzbot instead of the Reported-by tag
(may work out of the box, but need to double check if we start to use
this):

Link: https://syzkaller.appspot.com/bug?extid=726dc8c62c3536431ceb

Or similarly this may work:

Reported-by: https://syzkaller.appspot.com/bug?extid=726dc8c62c3536431ceb
I think the parsing code mostly looks for the hash.

This was proposed, but people said that they need links to lore and
don't want links to syzkaller dashboard. So this was rejected at the
time.

Theodore Ts'o

unread,
May 8, 2023, 5:06:13 AM5/8/23
to Dmitry Vyukov, Tudor Ambarus, Herbert Xu, syzbot, da...@davemloft.net, linux-...@vger.kernel.org, linux-...@vger.kernel.org, oli...@selenic.com, syzkall...@googlegroups.com, Jason Wang, Michael S. Tsirkin, Laurent Vivier, Rusty Russell, Aleksandr Nogikh, syzkaller
On Mon, May 08, 2023 at 07:33:39AM +0200, Dmitry Vyukov wrote:
> A link like this may work for syzbot instead of the Reported-by tag
> (may work out of the box, but need to double check if we start to use
> this):
>
> Link: https://syzkaller.appspot.com/bug?extid=726dc8c62c3536431ceb
>
> Or similarly this may work:
>
> Reported-by: https://syzkaller.appspot.com/bug?extid=726dc8c62c3536431ceb
> I think the parsing code mostly looks for the hash.
>
> This was proposed, but people said that they need links to lore and
> don't want links to syzkaller dashboard. So this was rejected at the
> time.

I think the "Reported-by: " line should continue to contain an e-mail,
since that way "git send-email" will automatically include a Cc: to
the mailing list address so that the syzbot page for the report will
contain a link to the page.

What *would* be useful would be a search box on the top-level
https://syzkaller.appspot.com where you could either enter an e-mail
address like:

syzbot+726dc8...@syzkaller.appspotmail.com

or the syzbot report title e.g.:

KCSAN: data-race in random_recv_done / virtio_read (3)

or just a function name:

sys_quotactl_fd

The search box could just push the text to google.com with
"site:syzkaller.appspot.com", which should mostly do the right thing.

Also, it would also be nice if all of the URL links on the
syzkaller.appspot.com used the id form of the URL. That is, to use

https://syzkaller.appspot.com/bug?extid=6c73bd34311ee489dbf5

instead of:

https://syzkaller.appspot.com/bug?id=32c54626e170a6b327ca2c8ae4c1aea666a8c20b

The extid form of the URL is shorter, and having a consistency so that
the primary URL is the extid would reduce confusion. The web site
will need to continue to support the id form of the URL since there
are quite a few of those URL's in mailing list archives and git commit
descriptions.

It also would be useful if there was a way to translate from the extid
hash to the id hash, so that it's possible to search for the extid and
id forms of the URL --- since the URL aliasing means that for a
developer trying to do code archeology and web searches, that we need
to search for both URL forms for past syzbot reports. (But if we can
avoid the aliasing confusion moving forward, that would be **really**
nice.)

Cheers,

- Ted

Aleksandr Nogikh

unread,
May 11, 2023, 11:11:46 AM5/11/23
to Theodore Ts'o, Dmitry Vyukov, Tudor Ambarus, Herbert Xu, syzbot, da...@davemloft.net, linux-...@vger.kernel.org, linux-...@vger.kernel.org, oli...@selenic.com, syzkall...@googlegroups.com, Jason Wang, Michael S. Tsirkin, Laurent Vivier, Rusty Russell, syzkaller
Hi Ted,
Thanks for the suggestion! I've filed
https://github.com/google/syzkaller/issues/3892

>
> Also, it would also be nice if all of the URL links on the
> syzkaller.appspot.com used the id form of the URL. That is, to use
>
> https://syzkaller.appspot.com/bug?extid=6c73bd34311ee489dbf5
>
> instead of:
>
> https://syzkaller.appspot.com/bug?id=32c54626e170a6b327ca2c8ae4c1aea666a8c20b
>
> The extid form of the URL is shorter, and having a consistency so that
> the primary URL is the extid would reduce confusion. The web site
> will need to continue to support the id form of the URL since there
> are quite a few of those URL's in mailing list archives and git commit
> descriptions.
>
> It also would be useful if there was a way to translate from the extid
> hash to the id hash, so that it's possible to search for the extid and
> id forms of the URL --- since the URL aliasing means that for a
> developer trying to do code archeology and web searches, that we need
> to search for both URL forms for past syzbot reports. (But if we can
> avoid the aliasing confusion moving forward, that would be **really**
> nice.)

I've just sent a PR [1] so that URLs from bug lists on the web
dashboard use the extid= instead of the id= parameter. Hopefully this
will reduce the confusion.

[1] https://github.com/google/syzkaller/pull/3891

--
Aleksandr

>
> Cheers,
>
> - Ted
Reply all
Reply to author
Forward
0 new messages