From: Shameer Kolothum <[email protected]>
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 <[email protected]>
Signed-off-by: Shameer Kolothum <[email protected]>
Reviewed-by: Eric Auger <[email protected]>
Message-id: [email protected]
Signed-off-by: Peter Maydell <[email protected]>
---
hw/arm/smmuv3-accel-stubs.c | 6 +++---
hw/arm/smmuv3-accel.c | 30 ++++++++++++++++++++----------
hw/arm/smmuv3-accel.h | 4 ++--
hw/arm/tegra241-cmdqv.c | 11 ++++++++---
4 files changed, 33 insertions(+), 18 deletions(-)
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 5e83dbd8ee..91aca1aa14 100644
--- a/hw/arm/smmuv3-accel.c
+++ b/hw/arm/smmuv3-accel.c
@@ -441,8 +441,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;
@@ -452,22 +457,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)) {
@@ -476,7 +481,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)
@@ -488,13 +493,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/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/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
