| Message ID | 20220526034851.683258-1-yajunw@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio/vhost-user: Fix wrong vhost notifier GPtrArray size | expand |
Yajun Wu <yajunw@nvidia.com> writes: > In fetch_or_create_notifier, idx begins with 0. So the GPtrArray size > should be idx + 1 and g_ptr_array_set_size should be called with idx + 1. > > This wrong GPtrArray size causes fetch_or_create_notifier return an invalid > address. Passing this invalid pointer to vhost_user_host_notifier_remove > causes assert fail: > > qemu/include/qemu/int128.h:27: int128_get64: Assertion `r == a' failed. > shutting down, reason=crashed > > Backends like dpdk-vdpa which sends out vhost notifier requests almost always > hit qemu crash. My bad. I was looking for ways to exercise this code but the internal tests didn't do it. I guess I should look at getting a test setup for dpdk. Anyway: Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > > Fixes: 503e355465 ("virtio/vhost-user: dynamically assign VhostUserHostNotifiers") > Signed-off-by: Yajun Wu <yajunw@nvidia.com> > Acked-by: Parav Pandit <parav@nvidia.com> > Change-Id: I87e0f7591ca9a59d210879b260704a2d9e9d6bcd > --- > hw/virtio/vhost-user.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index b040c1ad2b..dbc690d16c 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -1525,7 +1525,7 @@ static VhostUserHostNotifier *fetch_or_create_notifier(VhostUserState *u, > { > VhostUserHostNotifier *n = NULL; > if (idx >= u->notifiers->len) { > - g_ptr_array_set_size(u->notifiers, idx); > + g_ptr_array_set_size(u->notifiers, idx + 1); > } > > n = g_ptr_array_index(u->notifiers, idx);
Hi Michael, User space vhost clients are broken for few weeks now without this fix. With Alex's review, can you please merge it if there are no further comments? Thanks. -----Original Message----- From: Alex Bennée <alex.bennee@linaro.org> Sent: Thursday, May 26, 2022 3:09 PM To: Yajun Wu <yajunw@nvidia.com> Cc: qemu-devel@nongnu.org; mst@redhat.com; Parav Pandit <parav@nvidia.com> Subject: Re: [PATCH] virtio/vhost-user: Fix wrong vhost notifier GPtrArray size External email: Use caution opening links or attachments Yajun Wu <yajunw@nvidia.com> writes: > In fetch_or_create_notifier, idx begins with 0. So the GPtrArray size > should be idx + 1 and g_ptr_array_set_size should be called with idx + 1. > > This wrong GPtrArray size causes fetch_or_create_notifier return an > invalid address. Passing this invalid pointer to > vhost_user_host_notifier_remove causes assert fail: > > qemu/include/qemu/int128.h:27: int128_get64: Assertion `r == a' failed. > shutting down, reason=crashed > > Backends like dpdk-vdpa which sends out vhost notifier requests almost > always hit qemu crash. My bad. I was looking for ways to exercise this code but the internal tests didn't do it. I guess I should look at getting a test setup for dpdk. Anyway: Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > > Fixes: 503e355465 ("virtio/vhost-user: dynamically assign > VhostUserHostNotifiers") > Signed-off-by: Yajun Wu <yajunw@nvidia.com> > Acked-by: Parav Pandit <parav@nvidia.com> > Change-Id: I87e0f7591ca9a59d210879b260704a2d9e9d6bcd > --- > hw/virtio/vhost-user.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index > b040c1ad2b..dbc690d16c 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -1525,7 +1525,7 @@ static VhostUserHostNotifier > *fetch_or_create_notifier(VhostUserState *u, { > VhostUserHostNotifier *n = NULL; > if (idx >= u->notifiers->len) { > - g_ptr_array_set_size(u->notifiers, idx); > + g_ptr_array_set_size(u->notifiers, idx + 1); > } > > n = g_ptr_array_index(u->notifiers, idx); -- Alex Bennée
Reviewed-by: Eddie Dong <eddie.dong@intel.com> > -----Original Message----- > From: Qemu-devel <qemu-devel- > bounces+eddie.dong=intel.com@nongnu.org> On Behalf Of Yajun Wu > Sent: Wednesday, May 25, 2022 8:49 PM > To: qemu-devel@nongnu.org; mst@redhat.com; alex.bennee@linaro.org; > yajunw@nvidia.com > Cc: Parav Pandit <parav@nvidia.com> > Subject: [PATCH] virtio/vhost-user: Fix wrong vhost notifier GPtrArray size > > In fetch_or_create_notifier, idx begins with 0. So the GPtrArray size should > be idx + 1 and g_ptr_array_set_size should be called with idx + 1. > > This wrong GPtrArray size causes fetch_or_create_notifier return an invalid > address. Passing this invalid pointer to vhost_user_host_notifier_remove > causes assert fail: > > qemu/include/qemu/int128.h:27: int128_get64: Assertion `r == a' failed. > shutting down, reason=crashed > > Backends like dpdk-vdpa which sends out vhost notifier requests almost > always hit qemu crash. > > Fixes: 503e355465 ("virtio/vhost-user: dynamically assign > VhostUserHostNotifiers") > Signed-off-by: Yajun Wu <yajunw@nvidia.com> > Acked-by: Parav Pandit <parav@nvidia.com> > Change-Id: I87e0f7591ca9a59d210879b260704a2d9e9d6bcd > --- > hw/virtio/vhost-user.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index > b040c1ad2b..dbc690d16c 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -1525,7 +1525,7 @@ static VhostUserHostNotifier > *fetch_or_create_notifier(VhostUserState *u, { > VhostUserHostNotifier *n = NULL; > if (idx >= u->notifiers->len) { > - g_ptr_array_set_size(u->notifiers, idx); > + g_ptr_array_set_size(u->notifiers, idx + 1); > } > > n = g_ptr_array_index(u->notifiers, idx); > -- > 2.36.0 >
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index b040c1ad2b..dbc690d16c 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -1525,7 +1525,7 @@ static VhostUserHostNotifier *fetch_or_create_notifier(VhostUserState *u, { VhostUserHostNotifier *n = NULL; if (idx >= u->notifiers->len) { - g_ptr_array_set_size(u->notifiers, idx); + g_ptr_array_set_size(u->notifiers, idx + 1); } n = g_ptr_array_index(u->notifiers, idx);
In fetch_or_create_notifier, idx begins with 0. So the GPtrArray size should be idx + 1 and g_ptr_array_set_size should be called with idx + 1. This wrong GPtrArray size causes fetch_or_create_notifier return an invalid address. Passing this invalid pointer to vhost_user_host_notifier_remove causes assert fail: qemu/include/qemu/int128.h:27: int128_get64: Assertion `r == a' failed. shutting down, reason=crashed Backends like dpdk-vdpa which sends out vhost notifier requests almost always hit qemu crash. Fixes: 503e355465 ("virtio/vhost-user: dynamically assign VhostUserHostNotifiers") Signed-off-by: Yajun Wu <yajunw@nvidia.com> Acked-by: Parav Pandit <parav@nvidia.com> Change-Id: I87e0f7591ca9a59d210879b260704a2d9e9d6bcd --- hw/virtio/vhost-user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)