Hi Shameer,
On 6/23/26 9:51 AM, 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. Return false instead and guard
> warn_report_err_once() against NULL Error in both callers.
>
> 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]>
> ---
> hw/arm/smmuv3-accel.c | 6 ++++--
> hw/arm/tegra241-cmdqv.c | 4 +++-
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index 80900c2521..6d402d2ba6 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c
> @@ -451,7 +451,7 @@ 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 false;
> }
> error_setg(errp, "vEVENTQ(type %u id %u): read failed (%m)", type,
> id);
> return false;
> @@ -491,7 +491,9 @@ static void smmuv3_accel_event_read(void *opaque)
> if (!smmuv3_accel_event_read_validate(veventq,
> IOMMU_VEVENTQ_TYPE_ARM_SMMUV3,
> &buf,
> sizeof(buf), &local_err)) {
> - warn_report_err_once(local_err);
> + if (local_err) {
> + warn_report_err_once(local_err);
> + }
> return;
> }
> smmuv3_propagate_event(s, (Evt *)&buf.vevent);
> diff --git a/hw/arm/tegra241-cmdqv.c b/hw/arm/tegra241-cmdqv.c
> index 29c488e0e4..bb0b417e90 100644
> --- a/hw/arm/tegra241-cmdqv.c
> +++ b/hw/arm/tegra241-cmdqv.c
> @@ -845,7 +845,9 @@ static void tegra241_cmdqv_event_read(void *opaque)
> if (!smmuv3_accel_event_read_validate(veventq,
> IOMMU_VEVENTQ_TYPE_TEGRA241_CMDQV,
> &buf, sizeof(buf), &local_err)) {
> - warn_report_err_once(local_err);
> + if (local_err) {
This is not aligned with the general policy. If a function taking an
error handle fails (and returns false), it should set errp.
Don't you have a way to initialize buf in smmuv3_accel_event_read() and
only call smmuv3_propagate_event() if the hdr of vevnt is checked valid?
Thanks
Eric
> + warn_report_err_once(local_err);
> + }
> return;
> }
>
