On Tue, 23 Jun 2026 at 09:32, Eric Auger <[email protected]> wrote: > > 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?
I think fundamentally the function has three return states: - success - failure - try again later Probably the simplest thing is to not use a 'bool' for the return type but instead something that has 3 values. Then you can set errp on the "failure" case, and the caller can tell which of the three cases it is dealing with. -- PMM
