1. Patchew
  2. QEMU
  3. View series

[PATCH v2] hw/arm/smmuv3-accel: Fix veventq read returning true on EAGAIN/EINTR

Shameer Kolothum posted 1 patch 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260625122843.107584-1-skolothumtho@nvidia.com
Maintainers: Eric Auger <eric.auger@redhat.com>, Peter Maydell <peter.maydell@linaro.org>
hw/arm/smmuv3-accel.h       |  4 ++--
hw/arm/smmuv3-accel-stubs.c |  6 +++---
hw/arm/smmuv3-accel.c       | 30 ++++++++++++++++++++----------
hw/arm/tegra241-cmdqv.c     | 11 ++++++++---
4 files changed, 33 insertions(+), 18 deletions(-)
[PATCH v2] hw/arm/smmuv3-accel: Fix veventq read returning true on EAGAIN/EINTR
Posted by Shameer Kolothum 1 month ago 
smmuv3_accel_event_read_validate() returns true for EAGAIN/EINTR, but
no data has been read into the buffer. Callers treat true as success and
proceed to use the uninitialized buffer.

Change the return type to int with three distinct states:
  0  — success, buf is populated and valid
  1  — EAGAIN/EINTR, no data available
 -1  — error, @errp set

Resolves: Coverity CID 1660057
Fixes: d4aea0f75b ("hw/arm/smmuv3-accel: Introduce common helper for veventq read")
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
 v1 --> v2:
 -Changed helper ret type based on discussion here:
  https://lore.kernel.org/qemu-devel/20260623075101.17770-1-skolothumtho@nvidia.com/

Thanks
---
 hw/arm/smmuv3-accel.h       |  4 ++--
 hw/arm/smmuv3-accel-stubs.c |  6 +++---
 hw/arm/smmuv3-accel.c       | 30 ++++++++++++++++++++----------
 hw/arm/tegra241-cmdqv.c     | 11 ++++++++---
 4 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h
index dd755c394d..ea11d513cc 100644
--- a/hw/arm/smmuv3-accel.h
+++ b/hw/arm/smmuv3-accel.h
@@ -97,8 +97,8 @@ bool smmuv3_accel_issue_inv_cmd(SMMUv3State *s, void *cmd, SMMUDevice *sdev,
                                 Error **errp);
 void smmuv3_accel_idr_override(SMMUv3State *s);
 bool smmuv3_accel_alloc_veventq(SMMUv3State *s, Error **errp);
-bool smmuv3_accel_event_read_validate(IOMMUFDVeventq *veventq, uint32_t type,
-                                      void *buf, size_t size, Error **errp);
+int smmuv3_accel_event_read_validate(IOMMUFDVeventq *veventq, uint32_t type,
+                                     void *buf, size_t size, Error **errp);
 void smmuv3_accel_reset(SMMUv3State *s);
 SMMUv3AccelCmdqvType smmuv3_accel_cmdqv_type(Object *obj);
 
diff --git a/hw/arm/smmuv3-accel-stubs.c b/hw/arm/smmuv3-accel-stubs.c
index 147ae06163..b8dd7e7b89 100644
--- a/hw/arm/smmuv3-accel-stubs.c
+++ b/hw/arm/smmuv3-accel-stubs.c
@@ -47,10 +47,10 @@ bool smmuv3_accel_alloc_veventq(SMMUv3State *s, Error **errp)
     return true;
 }
 
-bool smmuv3_accel_event_read_validate(IOMMUFDVeventq *veventq, uint32_t type,
-                                      void *buf, size_t size, Error **errp)
+int smmuv3_accel_event_read_validate(IOMMUFDVeventq *veventq, uint32_t type,
+                                     void *buf, size_t size, Error **errp)
 {
-    return true;
+    return 0;
 }
 
 
diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
index 80900c2521..6df19f0e0e 100644
--- a/hw/arm/smmuv3-accel.c
+++ b/hw/arm/smmuv3-accel.c
@@ -440,8 +440,13 @@ bool smmuv3_accel_issue_inv_cmd(SMMUv3State *bs, void *cmd, SMMUDevice *sdev,
                    sizeof(Cmd), &entry_num, cmd, errp);
 }
 
-bool smmuv3_accel_event_read_validate(IOMMUFDVeventq *veventq, uint32_t type,
-                                      void *buf, size_t size, Error **errp)
+/*
+ * Returns  0 on success (buf is populated and valid).
+ * Returns  1 if the read should be retried (EAGAIN/EINTR).
+ * Returns -1 on error with @errp set.
+ */
+int smmuv3_accel_event_read_validate(IOMMUFDVeventq *veventq, uint32_t type,
+                                     void *buf, size_t size, Error **errp)
 {
     uint32_t last_seq = veventq->last_event_seq;
     uint32_t id = veventq->veventq_id;
@@ -451,22 +456,22 @@ bool smmuv3_accel_event_read_validate(IOMMUFDVeventq *veventq, uint32_t type,
     bytes = read(veventq->veventq_fd, buf, size);
     if (bytes <= 0) {
         if (errno == EAGAIN || errno == EINTR) {
-            return true;
+            return 1;
         }
         error_setg(errp, "vEVENTQ(type %u id %u): read failed (%m)", type, id);
-        return false;
+        return -1;
     }
     hdr = (struct iommufd_vevent_header *)buf;
     if (bytes == sizeof(*hdr) &&
         (hdr->flags & IOMMU_VEVENTQ_FLAG_LOST_EVENTS)) {
         error_setg(errp, "vEVENTQ(type %u id %u): overflowed", type, id);
         veventq->event_start = false;
-        return false;
+        return -1;
     }
     if (bytes < size) {
         error_setg(errp, "vEVENTQ(type %u id %u): short read(%zd/%zd bytes)",
                           type, id, bytes, size);
-        return false;
+        return -1;
     }
     /* Check sequence in hdr for lost events if any */
     if (veventq->event_start && (hdr->sequence - last_seq != 1)) {
@@ -475,7 +480,7 @@ bool smmuv3_accel_event_read_validate(IOMMUFDVeventq *veventq, uint32_t type,
     }
     veventq->last_event_seq = hdr->sequence;
     veventq->event_start = true;
-    return true;
+    return 0;
 }
 
 static void smmuv3_accel_event_read(void *opaque)
@@ -487,13 +492,18 @@ static void smmuv3_accel_event_read(void *opaque)
         struct iommu_vevent_arm_smmuv3 vevent;
     } buf;
     Error *local_err = NULL;
+    int ret;
 
-    if (!smmuv3_accel_event_read_validate(veventq,
-                                          IOMMU_VEVENTQ_TYPE_ARM_SMMUV3, &buf,
-                                          sizeof(buf), &local_err)) {
+    ret = smmuv3_accel_event_read_validate(veventq,
+                                           IOMMU_VEVENTQ_TYPE_ARM_SMMUV3, &buf,
+                                           sizeof(buf), &local_err);
+    if (ret < 0) {
         warn_report_err_once(local_err);
         return;
     }
+    if (ret > 0) {
+        return; /* EAGAIN/EINTR */
+    }
     smmuv3_propagate_event(s, (Evt *)&buf.vevent);
 }
 
diff --git a/hw/arm/tegra241-cmdqv.c b/hw/arm/tegra241-cmdqv.c
index 29c488e0e4..7223aa9d1d 100644
--- a/hw/arm/tegra241-cmdqv.c
+++ b/hw/arm/tegra241-cmdqv.c
@@ -841,13 +841,18 @@ static void tegra241_cmdqv_event_read(void *opaque)
         struct iommu_vevent_tegra241_cmdqv vevent;
     } buf;
     Error *local_err = NULL;
+    int ret;
 
-    if (!smmuv3_accel_event_read_validate(veventq,
-                                          IOMMU_VEVENTQ_TYPE_TEGRA241_CMDQV,
-                                          &buf, sizeof(buf), &local_err)) {
+    ret = smmuv3_accel_event_read_validate(veventq,
+                                           IOMMU_VEVENTQ_TYPE_TEGRA241_CMDQV,
+                                           &buf, sizeof(buf), &local_err);
+    if (ret < 0) {
         warn_report_err_once(local_err);
         return;
     }
+    if (ret > 0) {
+        return; /* EAGAIN/EINTR */
+    }
 
     if (buf.vevent.lvcmdq_err_map[0] || buf.vevent.lvcmdq_err_map[1]) {
         cmdqv->vintf_cmdq_err_map[0] =
-- 
2.43.0
Re: [PATCH v2] hw/arm/smmuv3-accel: Fix veventq read returning true on EAGAIN/EINTR
Posted by Eric Auger 3 weeks, 5 days ago 

On 6/25/26 2:28 PM, Shameer Kolothum wrote:
> smmuv3_accel_event_read_validate() returns true for EAGAIN/EINTR, but
> no data has been read into the buffer. Callers treat true as success and
> proceed to use the uninitialized buffer.
>
> Change the return type to int with three distinct states:
>   0  — success, buf is populated and valid
>   1  — EAGAIN/EINTR, no data available
>  -1  — error, @errp set
>
> Resolves: Coverity CID 1660057
> Fixes: d4aea0f75b ("hw/arm/smmuv3-accel: Introduce common helper for veventq read")
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
> ---
>  v1 --> v2:
>  -Changed helper ret type based on discussion here:
>   https://lore.kernel.org/qemu-devel/20260623075101.17770-1-skolothumtho@nvidia.com/
>
> Thanks
> ---
>  hw/arm/smmuv3-accel.h       |  4 ++--
>  hw/arm/smmuv3-accel-stubs.c |  6 +++---
>  hw/arm/smmuv3-accel.c       | 30 ++++++++++++++++++++----------
>  hw/arm/tegra241-cmdqv.c     | 11 ++++++++---
>  4 files changed, 33 insertions(+), 18 deletions(-)
>
> diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h
> index dd755c394d..ea11d513cc 100644
> --- a/hw/arm/smmuv3-accel.h
> +++ b/hw/arm/smmuv3-accel.h
> @@ -97,8 +97,8 @@ bool smmuv3_accel_issue_inv_cmd(SMMUv3State *s, void *cmd, SMMUDevice *sdev,
>                                  Error **errp);
>  void smmuv3_accel_idr_override(SMMUv3State *s);
>  bool smmuv3_accel_alloc_veventq(SMMUv3State *s, Error **errp);
> -bool smmuv3_accel_event_read_validate(IOMMUFDVeventq *veventq, uint32_t type,
> -                                      void *buf, size_t size, Error **errp);
> +int smmuv3_accel_event_read_validate(IOMMUFDVeventq *veventq, uint32_t type,
> +                                     void *buf, size_t size, Error **errp);
>  void smmuv3_accel_reset(SMMUv3State *s);
>  SMMUv3AccelCmdqvType smmuv3_accel_cmdqv_type(Object *obj);
>  
> diff --git a/hw/arm/smmuv3-accel-stubs.c b/hw/arm/smmuv3-accel-stubs.c
> index 147ae06163..b8dd7e7b89 100644
> --- a/hw/arm/smmuv3-accel-stubs.c
> +++ b/hw/arm/smmuv3-accel-stubs.c
> @@ -47,10 +47,10 @@ bool smmuv3_accel_alloc_veventq(SMMUv3State *s, Error **errp)
>      return true;
>  }
>  
> -bool smmuv3_accel_event_read_validate(IOMMUFDVeventq *veventq, uint32_t type,
> -                                      void *buf, size_t size, Error **errp)
> +int smmuv3_accel_event_read_validate(IOMMUFDVeventq *veventq, uint32_t type,
> +                                     void *buf, size_t size, Error **errp)
>  {
> -    return true;
> +    return 0;
>  }
>  
>  
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index 80900c2521..6df19f0e0e 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c
> @@ -440,8 +440,13 @@ bool smmuv3_accel_issue_inv_cmd(SMMUv3State *bs, void *cmd, SMMUDevice *sdev,
>                     sizeof(Cmd), &entry_num, cmd, errp);
>  }
>  
> -bool smmuv3_accel_event_read_validate(IOMMUFDVeventq *veventq, uint32_t type,
> -                                      void *buf, size_t size, Error **errp)
> +/*
> + * Returns  0 on success (buf is populated and valid).
> + * Returns  1 if the read should be retried (EAGAIN/EINTR).
> + * Returns -1 on error with @errp set.
> + */
> +int smmuv3_accel_event_read_validate(IOMMUFDVeventq *veventq, uint32_t type,
> +                                     void *buf, size_t size, Error **errp)
>  {
>      uint32_t last_seq = veventq->last_event_seq;
>      uint32_t id = veventq->veventq_id;
> @@ -451,22 +456,22 @@ bool smmuv3_accel_event_read_validate(IOMMUFDVeventq *veventq, uint32_t type,
>      bytes = read(veventq->veventq_fd, buf, size);
>      if (bytes <= 0) {
>          if (errno == EAGAIN || errno == EINTR) {
> -            return true;
> +            return 1;
>          }
>          error_setg(errp, "vEVENTQ(type %u id %u): read failed (%m)", type, id);
> -        return false;
> +        return -1;
>      }
>      hdr = (struct iommufd_vevent_header *)buf;
>      if (bytes == sizeof(*hdr) &&
>          (hdr->flags & IOMMU_VEVENTQ_FLAG_LOST_EVENTS)) {
>          error_setg(errp, "vEVENTQ(type %u id %u): overflowed", type, id);
>          veventq->event_start = false;
> -        return false;
> +        return -1;
>      }
>      if (bytes < size) {
>          error_setg(errp, "vEVENTQ(type %u id %u): short read(%zd/%zd bytes)",
>                            type, id, bytes, size);
> -        return false;
> +        return -1;
>      }
>      /* Check sequence in hdr for lost events if any */
>      if (veventq->event_start && (hdr->sequence - last_seq != 1)) {
> @@ -475,7 +480,7 @@ bool smmuv3_accel_event_read_validate(IOMMUFDVeventq *veventq, uint32_t type,
>      }
>      veventq->last_event_seq = hdr->sequence;
>      veventq->event_start = true;
> -    return true;
> +    return 0;
>  }
>  
>  static void smmuv3_accel_event_read(void *opaque)
> @@ -487,13 +492,18 @@ static void smmuv3_accel_event_read(void *opaque)
>          struct iommu_vevent_arm_smmuv3 vevent;
>      } buf;
>      Error *local_err = NULL;
> +    int ret;
>  
> -    if (!smmuv3_accel_event_read_validate(veventq,
> -                                          IOMMU_VEVENTQ_TYPE_ARM_SMMUV3, &buf,
> -                                          sizeof(buf), &local_err)) {
> +    ret = smmuv3_accel_event_read_validate(veventq,
> +                                           IOMMU_VEVENTQ_TYPE_ARM_SMMUV3, &buf,
> +                                           sizeof(buf), &local_err);
> +    if (ret < 0) {
>          warn_report_err_once(local_err);
>          return;
>      }
> +    if (ret > 0) {
> +        return; /* EAGAIN/EINTR */
> +    }
>      smmuv3_propagate_event(s, (Evt *)&buf.vevent);
>  }
>  
> diff --git a/hw/arm/tegra241-cmdqv.c b/hw/arm/tegra241-cmdqv.c
> index 29c488e0e4..7223aa9d1d 100644
> --- a/hw/arm/tegra241-cmdqv.c
> +++ b/hw/arm/tegra241-cmdqv.c
> @@ -841,13 +841,18 @@ static void tegra241_cmdqv_event_read(void *opaque)
>          struct iommu_vevent_tegra241_cmdqv vevent;
>      } buf;
>      Error *local_err = NULL;
> +    int ret;
>  
> -    if (!smmuv3_accel_event_read_validate(veventq,
> -                                          IOMMU_VEVENTQ_TYPE_TEGRA241_CMDQV,
> -                                          &buf, sizeof(buf), &local_err)) {
> +    ret = smmuv3_accel_event_read_validate(veventq,
> +                                           IOMMU_VEVENTQ_TYPE_TEGRA241_CMDQV,
> +                                           &buf, sizeof(buf), &local_err);
> +    if (ret < 0) {
>          warn_report_err_once(local_err);
>          return;
>      }
> +    if (ret > 0) {
> +        return; /* EAGAIN/EINTR */
> +    }
>  
>      if (buf.vevent.lvcmdq_err_map[0] || buf.vevent.lvcmdq_err_map[1]) {
>          cmdqv->vintf_cmdq_err_map[0] =
Re: [PATCH v2] hw/arm/smmuv3-accel: Fix veventq read returning true on EAGAIN/EINTR
Posted by Peter Maydell 3 weeks, 2 days ago 
On Mon, 29 Jun 2026 at 16:36, Eric Auger <eric.auger@redhat.com> wrote:
>
>
>
> On 6/25/26 2:28 PM, Shameer Kolothum wrote:
> > smmuv3_accel_event_read_validate() returns true for EAGAIN/EINTR, but
> > no data has been read into the buffer. Callers treat true as success and
> > proceed to use the uninitialized buffer.
> >
> > Change the return type to int with three distinct states:
> >   0  — success, buf is populated and valid
> >   1  — EAGAIN/EINTR, no data available
> >  -1  — error, @errp set
> >
> > Resolves: Coverity CID 1660057
> > Fixes: d4aea0f75b ("hw/arm/smmuv3-accel: Introduce common helper for veventq read")
> > Reported-by: Peter Maydell <peter.maydell@linaro.org>
> > Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
>
> Thanks



Applied to target-arm.next, thanks.

-- PMM

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.