On 12/21/23 18:14, Eric Auger wrote:
Hi Cédric,
On 12/21/23 17:00, Cédric Le Goater wrote:
[ ... ]
+static void iommufd_backend_init(Object *obj)
+{
+ IOMMUFDBackend *be = IOMMUFD_BACKEND(obj);
+
+ be->fd = -1;
+ be->users = 0;
+ be->owned = true;
+ qemu_mutex_init(&be->lock);> +}
+
+static void iommufd_backend_finalize(Object *obj)
+{
+ IOMMUFDBackend *be = IOMMUFD_BACKEND(obj);
+
+ if (be->owned) {
+ close(be->fd);
+ be->fd = -1;
+ }
+}
+
+static void iommufd_backend_set_fd(Object *obj, const char *str,
Error **errp)
+{
+ IOMMUFDBackend *be = IOMMUFD_BACKEND(obj);
+ int fd = -1;
+
+ fd = monitor_fd_param(monitor_cur(), str, errp);
+ if (fd == -1) {
+ error_prepend(errp, "Could not parse remote object fd %s:",
str);
+ return;
+ }
+ qemu_mutex_lock(&be->lock);
+ be->fd = fd;
+ be->owned = false;
+ qemu_mutex_unlock(&be->lock);
+ trace_iommu_backend_set_fd(be->fd);
+}
+
+static bool iommufd_backend_can_be_deleted(UserCreatable *uc)
+{
+ IOMMUFDBackend *be = IOMMUFD_BACKEND(uc);
+
+ return !be->users;
Coverity CID 1531549 reports a concurrent data access violation because
be->users is being accessed without holding the mutex.
I wonder how useful is this mutex anyhow, since the code paths should
be protected by the BQL lock. If you agree, I will send an update to
simply drop be->lock and solve this report.
I am not totally comfortable with the fact BQL covers the same
protection? Please can you elaborate.
These routines are called when a device is created which is called
from the QEMU main thread which exits holding the BQL. It should be
fine.
Thanks,
C.
