2021-10-27 21:32:29

by Laurent Vivier

[permalink] [raw]
Subject: Re: [syzbot] KASAN: slab-out-of-bounds Read in copy_data

On 27/10/2021 19:03, Laurent Vivier wrote:
> On 27/10/2021 18:25, Laurent Vivier wrote:
>> On 27/10/2021 17:28, Michael S. Tsirkin wrote:
>>> On Wed, Oct 27, 2021 at 03:36:19PM +0200, Dmitry Vyukov wrote:
>>>> On Wed, 27 Oct 2021 at 15:11, Laurent Vivier <[email protected]> wrote:
>>>>>
>>>>> On 26/10/2021 18:39, syzbot wrote:
>>>>>> Hello,
>>>>>>
>>>>>> syzbot found the following issue on:
>>>>>>
>>>>>> HEAD commit:    9ae1fbdeabd3 Add linux-next specific files for 20211025
>>>>>> git tree:       linux-next
>>>>>> console output: https://syzkaller.appspot.com/x/log.txt?x=1331363cb00000
>>>>>> kernel config:  https://syzkaller.appspot.com/x/.config?x=aeb17e42bc109064
>>>>>> dashboard link: https://syzkaller.appspot.com/bug?extid=b86736b5935e0d25b446
>>>>>> compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for
>>>>>> Debian) 2.35.2
>>>>>> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=116ce954b00000
>>>>>> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=132fcf62b00000
>>>>>>
>>>>>> The issue was bisected to:
>>>>>>
>>>>>> commit 22849b5ea5952d853547cc5e0651f34a246b2a4f
>>>>>> Author: Leon Romanovsky <[email protected]>
>>>>>> Date:   Thu Oct 21 14:16:14 2021 +0000
>>>>>>
>>>>>>       devlink: Remove not-executed trap policer notifications
>>>>>>
>>>>>> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=137d8bfcb00000
>>>>>> final oops:     https://syzkaller.appspot.com/x/report.txt?x=10fd8bfcb00000
>>>>>> console output: https://syzkaller.appspot.com/x/log.txt?x=177d8bfcb00000
>>>>>>
>>>>>> IMPORTANT: if you fix the issue, please add the following tag to the commit:
>>>>>> Reported-by: [email protected]
>>>>>> Fixes: 22849b5ea595 ("devlink: Remove not-executed trap policer notifications")
>>>>>>
>>>>>> ==================================================================
>>>>>> BUG: KASAN: slab-out-of-bounds in memcpy include/linux/fortify-string.h:225 [inline]
>>>>>> BUG: KASAN: slab-out-of-bounds in copy_data+0xf3/0x2e0
>>>>>> drivers/char/hw_random/virtio-rng.c:68
>>>>>> Read of size 64 at addr ffff88801a7a1580 by task syz-executor989/6542
>>>>>>
>>>>>
>>>>> I'm not able to reproduce the problem with next-20211026 and the C reproducer.
>>>>>
>>>>> And reviewing the code in copy_data() I don't see any issue.
>>>>>
>>>>> Is it possible to know what it the VM configuration used to test it?
>>>>
>>>> Hi Laurent,
>>>>
>>>> syzbot used e2-standard-2 GCE VM when that happened.
>>>> You can see some info about these VMs under the "VM info" link on the dashboard.
>>>
>>> Could you pls confirm whether reverting
>>> caaf2874ba27b92bca6f0298bf88bad94067ec37 addresses this?
>>>
>>
>> I've restarted the syzbot on top of "hwrng: virtio - don't wait on cleanup" [1] and the
>> problem has not been triggered.
>>
>> See https://syzkaller.appspot.com/bug?extid=b86736b5935e0d25b446
>
> The problem seems to be introduced by the last patch:
>
> "hwrng: virtio - always add a pending request"

I think I understand the problem.

As we check data_avail != 0 before waiting on the completion, we can have a data_idx != 0.

The following change fixes the problem for me:

--- a/drivers/char/hw_random/virtio-rng.c
+++ b/drivers/char/hw_random/virtio-rng.c
@@ -52,6 +52,8 @@ 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));


MST, do you update the patch or do you want I send a new version?

Thanks,
Laurent