* [PATCH v2] vhost: add runtime locking check in unsafe APIs
@ 2022-05-11 6:58 Maxime Coquelin
2022-05-11 7:35 ` David Marchand
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Maxime Coquelin @ 2022-05-11 6:58 UTC (permalink / raw)
To: dev, chenbo.xia, xuan.ding, jiayu.hu, cheng1.jiang, sunil.pai.g,
david.marchand
Cc: Maxime Coquelin
This patch adds runtime checks in unsafe Vhost async APIs,
to ensure the access lock is taken.
The detection won't work every time, as another thread
could take the lock, but it would help to detect misuse
of these unsafe API.
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
lib/vhost/vhost.c | 18 ++++++++++++++++++
lib/vhost/virtio_net.c | 6 ++++++
2 files changed, 24 insertions(+)
diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
index df0bb9d043..39cbeb415c 100644
--- a/lib/vhost/vhost.c
+++ b/lib/vhost/vhost.c
@@ -1732,6 +1732,12 @@ rte_vhost_async_channel_register_thread_unsafe(int vid, uint16_t queue_id)
if (unlikely(vq == NULL || !dev->async_copy))
return -1;
+ if (unlikely(!rte_spinlock_is_locked(&vq->access_lock))) {
+ VHOST_LOG_CONFIG(ERR, "(%s) %s() called without access lock taken.\n",
+ dev->ifname, __func__);
+ return -1;
+ }
+
return async_channel_register(vid, queue_id);
}
@@ -1796,6 +1802,12 @@ rte_vhost_async_channel_unregister_thread_unsafe(int vid, uint16_t queue_id)
if (vq == NULL)
return -1;
+ if (unlikely(!rte_spinlock_is_locked(&vq->access_lock))) {
+ VHOST_LOG_CONFIG(ERR, "(%s) %s() called without access lock taken.\n",
+ dev->ifname, __func__);
+ return -1;
+ }
+
if (!vq->async)
return 0;
@@ -1925,6 +1937,12 @@ rte_vhost_async_get_inflight_thread_unsafe(int vid, uint16_t queue_id)
if (vq == NULL)
return ret;
+ if (unlikely(!rte_spinlock_is_locked(&vq->access_lock))) {
+ VHOST_LOG_CONFIG(ERR, "(%s) %s() called without access lock taken.\n",
+ dev->ifname, __func__);
+ return -1;
+ }
+
if (!vq->async)
return ret;
diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index 5f432b0d77..59163bc165 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -2091,6 +2091,12 @@ rte_vhost_clear_queue_thread_unsafe(int vid, uint16_t queue_id,
vq = dev->virtqueue[queue_id];
+ if (unlikely(!rte_spinlock_is_locked(&vq->access_lock))) {
+ VHOST_LOG_DATA(ERR, "(%s) %s() called without access lock taken.\n",
+ dev->ifname, __func__);
+ return -1;
+ }
+
if (unlikely(!vq->async)) {
VHOST_LOG_DATA(ERR, "(%s) %s: async not registered for queue id %d.\n",
dev->ifname, __func__, queue_id);
--
2.35.1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] vhost: add runtime locking check in unsafe APIs
2022-05-11 6:58 [PATCH v2] vhost: add runtime locking check in unsafe APIs Maxime Coquelin
@ 2022-05-11 7:35 ` David Marchand
2022-05-11 7:49 ` Hu, Jiayu
2022-05-11 13:58 ` Xia, Chenbo
2022-05-17 13:23 ` Maxime Coquelin
2 siblings, 1 reply; 5+ messages in thread
From: David Marchand @ 2022-05-11 7:35 UTC (permalink / raw)
To: Maxime Coquelin
Cc: dev, Xia, Chenbo, Xuan Ding, Jiayu Hu, Cheng Jiang, Sunil Pai G
On Wed, May 11, 2022 at 8:58 AM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
> This patch adds runtime checks in unsafe Vhost async APIs,
> to ensure the access lock is taken.
>
> The detection won't work every time, as another thread
> could take the lock, but it would help to detect misuse
> of these unsafe API.
>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
--
David Marchand
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v2] vhost: add runtime locking check in unsafe APIs
2022-05-11 7:35 ` David Marchand
@ 2022-05-11 7:49 ` Hu, Jiayu
0 siblings, 0 replies; 5+ messages in thread
From: Hu, Jiayu @ 2022-05-11 7:49 UTC (permalink / raw)
To: David Marchand, Maxime Coquelin
Cc: dev, Xia, Chenbo, Ding, Xuan, Jiang, Cheng1, Pai G, Sunil
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Wednesday, May 11, 2022 3:35 PM
> To: Maxime Coquelin <maxime.coquelin@redhat.com>
> Cc: dev <dev@dpdk.org>; Xia, Chenbo <chenbo.xia@intel.com>; Ding, Xuan
> <xuan.ding@intel.com>; Hu, Jiayu <jiayu.hu@intel.com>; Jiang, Cheng1
> <cheng1.jiang@intel.com>; Pai G, Sunil <sunil.pai.g@intel.com>
> Subject: Re: [PATCH v2] vhost: add runtime locking check in unsafe APIs
>
> On Wed, May 11, 2022 at 8:58 AM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
> >
> > This patch adds runtime checks in unsafe Vhost async APIs, to ensure
> > the access lock is taken.
> >
> > The detection won't work every time, as another thread could take the
> > lock, but it would help to detect misuse of these unsafe API.
> >
> > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>
> Reviewed-by: David Marchand <david.marchand@redhat.com>
Reviewed-by: Jiayu Hu <jiayu.hu@intel.com>
Thanks,
Jiayu
>
>
> --
> David Marchand
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v2] vhost: add runtime locking check in unsafe APIs
2022-05-11 6:58 [PATCH v2] vhost: add runtime locking check in unsafe APIs Maxime Coquelin
2022-05-11 7:35 ` David Marchand
@ 2022-05-11 13:58 ` Xia, Chenbo
2022-05-17 13:23 ` Maxime Coquelin
2 siblings, 0 replies; 5+ messages in thread
From: Xia, Chenbo @ 2022-05-11 13:58 UTC (permalink / raw)
To: Maxime Coquelin, dev, Ding, Xuan, Hu, Jiayu, Jiang, Cheng1,
Pai G, Sunil, david.marchand
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Wednesday, May 11, 2022 2:59 PM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Ding, Xuan
> <xuan.ding@intel.com>; Hu, Jiayu <jiayu.hu@intel.com>; Jiang, Cheng1
> <cheng1.jiang@intel.com>; Pai G, Sunil <sunil.pai.g@intel.com>;
> david.marchand@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v2] vhost: add runtime locking check in unsafe APIs
>
> This patch adds runtime checks in unsafe Vhost async APIs,
> to ensure the access lock is taken.
>
> The detection won't work every time, as another thread
> could take the lock, but it would help to detect misuse
> of these unsafe API.
>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
> lib/vhost/vhost.c | 18 ++++++++++++++++++
> lib/vhost/virtio_net.c | 6 ++++++
> 2 files changed, 24 insertions(+)
>
> diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
> index df0bb9d043..39cbeb415c 100644
> --- a/lib/vhost/vhost.c
> +++ b/lib/vhost/vhost.c
> @@ -1732,6 +1732,12 @@ rte_vhost_async_channel_register_thread_unsafe(int
> vid, uint16_t queue_id)
> if (unlikely(vq == NULL || !dev->async_copy))
> return -1;
>
> + if (unlikely(!rte_spinlock_is_locked(&vq->access_lock))) {
> + VHOST_LOG_CONFIG(ERR, "(%s) %s() called without access lock
> taken.\n",
> + dev->ifname, __func__);
> + return -1;
> + }
> +
> return async_channel_register(vid, queue_id);
> }
>
> @@ -1796,6 +1802,12 @@
> rte_vhost_async_channel_unregister_thread_unsafe(int vid, uint16_t
> queue_id)
> if (vq == NULL)
> return -1;
>
> + if (unlikely(!rte_spinlock_is_locked(&vq->access_lock))) {
> + VHOST_LOG_CONFIG(ERR, "(%s) %s() called without access lock
> taken.\n",
> + dev->ifname, __func__);
> + return -1;
> + }
> +
> if (!vq->async)
> return 0;
>
> @@ -1925,6 +1937,12 @@ rte_vhost_async_get_inflight_thread_unsafe(int vid,
> uint16_t queue_id)
> if (vq == NULL)
> return ret;
>
> + if (unlikely(!rte_spinlock_is_locked(&vq->access_lock))) {
> + VHOST_LOG_CONFIG(ERR, "(%s) %s() called without access lock
> taken.\n",
> + dev->ifname, __func__);
> + return -1;
> + }
> +
> if (!vq->async)
> return ret;
>
> diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
> index 5f432b0d77..59163bc165 100644
> --- a/lib/vhost/virtio_net.c
> +++ b/lib/vhost/virtio_net.c
> @@ -2091,6 +2091,12 @@ rte_vhost_clear_queue_thread_unsafe(int vid,
> uint16_t queue_id,
>
> vq = dev->virtqueue[queue_id];
>
> + if (unlikely(!rte_spinlock_is_locked(&vq->access_lock))) {
> + VHOST_LOG_DATA(ERR, "(%s) %s() called without access lock
> taken.\n",
> + dev->ifname, __func__);
> + return -1;
> + }
> +
> if (unlikely(!vq->async)) {
> VHOST_LOG_DATA(ERR, "(%s) %s: async not registered for queue
> id %d.\n",
> dev->ifname, __func__, queue_id);
> --
> 2.35.1
Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] vhost: add runtime locking check in unsafe APIs
2022-05-11 6:58 [PATCH v2] vhost: add runtime locking check in unsafe APIs Maxime Coquelin
2022-05-11 7:35 ` David Marchand
2022-05-11 13:58 ` Xia, Chenbo
@ 2022-05-17 13:23 ` Maxime Coquelin
2 siblings, 0 replies; 5+ messages in thread
From: Maxime Coquelin @ 2022-05-17 13:23 UTC (permalink / raw)
To: dev, chenbo.xia, xuan.ding, jiayu.hu, cheng1.jiang, sunil.pai.g,
david.marchand
On 5/11/22 08:58, Maxime Coquelin wrote:
> This patch adds runtime checks in unsafe Vhost async APIs,
> to ensure the access lock is taken.
>
> The detection won't work every time, as another thread
> could take the lock, but it would help to detect misuse
> of these unsafe API.
>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
> lib/vhost/vhost.c | 18 ++++++++++++++++++
> lib/vhost/virtio_net.c | 6 ++++++
> 2 files changed, 24 insertions(+)
>
Applied to dpdk-next-virtio/main.
Thanks,
Maxime
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-17 13:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-11 6:58 [PATCH v2] vhost: add runtime locking check in unsafe APIs Maxime Coquelin
2022-05-11 7:35 ` David Marchand
2022-05-11 7:49 ` Hu, Jiayu
2022-05-11 13:58 ` Xia, Chenbo
2022-05-17 13:23 ` Maxime Coquelin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).