From: Anirudh Rayabharam <mail@anirudhrb.com>
To: Stefano Garzarella <sgarzare@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>,
kernel list <linux-kernel@vger.kernel.org>,
Mike Christie <michael.christie@oracle.com>,
Jason Wang <jasowang@redhat.com>, netdev <netdev@vger.kernel.org>,
Linux Virtualization <virtualization@lists.linux-foundation.org>,
Stefan Hajnoczi <stefanha@redhat.com>, kvm <kvm@vger.kernel.org>,
Hillf Danton <hdanton@sina.com>
Subject: Re: [PATCH] vhost/vsock: don't check owner in vhost_vsock_stop() while releasing
Date: Tue, 22 Feb 2022 01:06:12 +0530 [thread overview]
Message-ID: <YhPprNUAqYS3RVtU@anirudhrb.com> (raw)
In-Reply-To: <20220221182628.vy2bjntxnzqh7elj@sgarzare-redhat>
On Mon, Feb 21, 2022 at 07:26:28PM +0100, Stefano Garzarella wrote:
> On Mon, Feb 21, 2022 at 11:33:11PM +0530, Anirudh Rayabharam wrote:
> > On Mon, Feb 21, 2022 at 05:44:20PM +0100, Stefano Garzarella wrote:
> > > On Mon, Feb 21, 2022 at 09:44:39PM +0530, Anirudh Rayabharam wrote:
> > > > On Mon, Feb 21, 2022 at 02:59:30PM +0100, Stefano Garzarella wrote:
> > > > > On Mon, Feb 21, 2022 at 12:49 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
> > > > > >
> > > > > > vhost_vsock_stop() calls vhost_dev_check_owner() to check the device
> > > > > > ownership. It expects current->mm to be valid.
> > > > > >
> > > > > > vhost_vsock_stop() is also called by vhost_vsock_dev_release() when
> > > > > > the user has not done close(), so when we are in do_exit(). In this
> > > > > > case current->mm is invalid and we're releasing the device, so we
> > > > > > should clean it anyway.
> > > > > >
> > > > > > Let's check the owner only when vhost_vsock_stop() is called
> > > > > > by an ioctl.
> > > > > >
> > > > > > Fixes: 433fc58e6bf2 ("VSOCK: Introduce vhost_vsock.ko")
> > > > > > Cc: stable@vger.kernel.org
> > > > > > Reported-by: syzbot+1e3ea63db39f2b4440e0@syzkaller.appspotmail.com
> > > > > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > > > > ---
> > > > > > drivers/vhost/vsock.c | 14 ++++++++------
> > > > > > 1 file changed, 8 insertions(+), 6 deletions(-)
> > > > >
> > > > > Reported-and-tested-by: syzbot+0abd373e2e50d704db87@syzkaller.appspotmail.com
> > > >
> > > > I don't think this patch fixes "INFO: task hung in vhost_work_dev_flush"
> > > > even though syzbot says so. I am able to reproduce the issue locally
> > > > even with this patch applied.
> > >
> > > Are you using the sysbot reproducer or another test?
> > > In that case, can you share it?
> >
> > I am using the syzbot reproducer.
> >
> > >
> > > From the stack trace it seemed to me that the worker accesses a zone that
> > > has been cleaned (iotlb), so it is invalid and fails.
> >
> > Would the thread hang in that case? How?
>
> Looking at this log [1] it seems that the process is blocked on the
> wait_for_completion() in vhost_work_dev_flush().
>
> Since we're not setting the backend to NULL to stop the worker, it's likely
> that the worker will keep running, preventing the flush work from
> completing.
The log shows that the worker thread is stuck in iotlb_access_ok(). How
will setting the backend to NULL stop it? During my debugging I found
that the worker is stuck in this while loop:
1361 while (len > s) {
1362 map = vhost_iotlb_itree_first(umem, addr, last);
1363 if (map == NULL || map->start > addr) {
1364 vhost_iotlb_miss(vq, addr, access);
1365 return false;
1366 } else if (!(map->perm & access)) {
1367 /* Report the possible access violation by
1368 * request another translation from userspace.
1369 */
1370 return false;
1371 }
1372
1373 pr_info("iotlb_access_ok: after msize=%llu, mstart=%llu\n",
1374 map->size, map->start);
1375 size = map->size - addr + map->start;
1376
1377 if (orig_addr == addr && size >= len)
1378 vhost_vq_meta_update(vq, map, type);
1379
1380 s += size;
1381 addr += size;
1382 }
>
> [1] https://syzkaller.appspot.com/text?tag=CrashLog&x=153f0852700000
>
next prev parent reply other threads:[~2022-02-21 19:36 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-21 11:49 [PATCH] vhost/vsock: don't check owner in vhost_vsock_stop() while releasing Stefano Garzarella
2022-02-21 11:49 ` Stefano Garzarella
2022-02-21 13:59 ` Stefano Garzarella
2022-02-21 13:59 ` Stefano Garzarella
2022-02-21 16:14 ` Anirudh Rayabharam
2022-02-21 16:44 ` Stefano Garzarella
2022-02-21 16:44 ` Stefano Garzarella
2022-02-21 18:03 ` Anirudh Rayabharam
2022-02-21 18:26 ` Stefano Garzarella
2022-02-21 18:26 ` Stefano Garzarella
2022-02-21 19:36 ` Anirudh Rayabharam [this message]
2022-02-22 9:05 ` Stefano Garzarella
2022-02-22 9:05 ` Stefano Garzarella
2022-02-21 15:03 ` Michael S. Tsirkin
2022-02-21 15:03 ` Michael S. Tsirkin
2022-02-21 15:22 ` Stefano Garzarella
2022-02-21 15:22 ` Stefano Garzarella
2022-02-21 23:51 ` kernel test robot
2022-02-22 5:30 ` Dan Carpenter
2022-02-22 5:30 ` Dan Carpenter
2022-02-22 5:30 ` Dan Carpenter
2022-02-22 8:11 ` Stefano Garzarella
2022-02-22 8:11 ` Stefano Garzarella
2022-02-22 8:11 ` Stefano Garzarella
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YhPprNUAqYS3RVtU@anirudhrb.com \
--to=mail@anirudhrb.com \
--cc=hdanton@sina.com \
--cc=jasowang@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=michael.christie@oracle.com \
--cc=mst@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=sgarzare@redhat.com \
--cc=stefanha@redhat.com \
--cc=virtualization@lists.linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.