Re: WARNING in __mmdrop

From: Jason Wang
Date: Tue Jul 23 2019 - 01:49:09 EST



On 2019/7/23 äå1:02, Michael S. Tsirkin wrote:
On Tue, Jul 23, 2019 at 11:55:28AM +0800, Jason Wang wrote:
On 2019/7/22 äå4:02, Michael S. Tsirkin wrote:
On Mon, Jul 22, 2019 at 01:21:59PM +0800, Jason Wang wrote:
On 2019/7/21 äå6:02, Michael S. Tsirkin wrote:
On Sat, Jul 20, 2019 at 03:08:00AM -0700, syzbot wrote:
syzbot has bisected this bug to:

commit 7f466032dc9e5a61217f22ea34b2df932786bbfc
Author: Jason Wang <jasowang@xxxxxxxxxx>
Date: Fri May 24 08:12:18 2019 +0000

vhost: access vq metadata through kernel virtual address

bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=149a8a20600000
start commit: 6d21a41b Add linux-next specific files for 20190718
git tree: linux-next
final crash: https://syzkaller.appspot.com/x/report.txt?x=169a8a20600000
console output: https://syzkaller.appspot.com/x/log.txt?x=129a8a20600000
kernel config: https://syzkaller.appspot.com/x/.config?x=3430a151e1452331
dashboard link: https://syzkaller.appspot.com/bug?extid=e58112d71f77113ddb7b
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10139e68600000

Reported-by: syzbot+e58112d71f77113ddb7b@xxxxxxxxxxxxxxxxxxxxxxxxx
Fixes: 7f466032dc9e ("vhost: access vq metadata through kernel virtual
address")

For information about bisection process see: https://goo.gl/tpsmEJ#bisection
OK I poked at this for a bit, I see several things that
we need to fix, though I'm not yet sure it's the reason for
the failures:


1. mmu_notifier_register shouldn't be called from vhost_vring_set_num_addr
That's just a bad hack,
This is used to avoid holding lock when checking whether the addresses are
overlapped. Otherwise we need to take spinlock for each invalidation request
even if it was the va range that is not interested for us. This will be very
slow e.g during guest boot.
KVM seems to do exactly that.
I tried and guest does not seem to boot any slower.
Do you observe any slowdown?

Yes I do.


Now I took a hard look at the uaddr hackery it really makes
me nervious. So I think for this release we want something
safe, and optimizations on top. As an alternative revert the
optimization and try again for next merge window.

Will post a series of fixes, let me know if you're ok with that.

Thanks
I'd prefer you to take a hard look at the patch I posted
which makes code cleaner,


I did. But it looks to me a series that is only about 60 lines of code can fix all the issues we found without reverting the uaddr optimization.


and ad optimizations on top.
But other ways could be ok too.


I'm waiting for the test result from syzbot and will post. Let's see if you are OK with that.

Thanks