On Fri, Jan 24, 2025 at 04:30:34PM -0800, Nicolin Chen wrote:

+	list_add_tail(&vevent->node, &eventq->deliver);
+	vevent->on_list = true;
+	vevent->header.sequence = atomic_read(&veventq->sequence);
+	if (atomic_read(&veventq->sequence) == INT_MAX)
+		atomic_set(&veventq->sequence, 0);
+	else
+		atomic_inc(&veventq->sequence);
+	spin_unlock(&eventq->lock);

This is all locked, we don't need veventq->sequence to be an atomic?

The bounding can be done with some simple math:

  veventq->sequence = (veventq->sequence + 1) & INT_MAX;

+static struct iommufd_vevent *
+iommufd_veventq_deliver_fetch(struct iommufd_veventq *veventq)
+{
+	struct iommufd_eventq *eventq = &veventq->common;
+	struct list_head *list = &eventq->deliver;
+	struct iommufd_vevent *vevent = NULL;
+
+	spin_lock(&eventq->lock);
+	if (!list_empty(list)) {
+		vevent = list_first_entry(list, struct iommufd_vevent, node);
+		list_del(&vevent->node);
+		vevent->on_list = false;
+	}
+	/* Make a copy of the overflow node for copy_to_user */
+	if (vevent == &veventq->overflow) {
+		vevent = kzalloc(sizeof(*vevent), GFP_ATOMIC);
+		if (vevent)
+			memcpy(vevent, &veventq->overflow, sizeof(*vevent));
+	}

This error handling is wonky, if we can't allocate then we shouldn't
have done the list_del. Just return NULL which will cause
iommufd_veventq_fops_read() to exist and userspace will try again.

Why? We are about to free the memory?

+int iommufd_veventq_alloc(struct iommufd_ucmd *ucmd)
+{
+	struct iommu_veventq_alloc *cmd = ucmd->cmd;
+	struct iommufd_veventq *veventq;
+	struct iommufd_viommu *viommu;
+	int fdno;
+	int rc;
+
+	if (cmd->flags || cmd->type == IOMMU_VEVENTQ_TYPE_DEFAULT)
+		return -EOPNOTSUPP;
+	if (!cmd->veventq_depth)
+		return -EINVAL;

Check __reserved for 0 too

Jason