arm_vsmmu_cache_invalidate() copies the entire user invalidation array into
one kernel allocation, then converts and submits those commands in batches
of CMDQ_BATCH_ENTRIES - 1. Sizing a single allocation from a user-supplied
count while tracking progress with a separate cursor is the root of several
independent problems:
- entry_num, the in/out count of successfully handled requests, is set to
"cur - cmds" on every error path. "cur" counts converted commands, not
submitted ones, so a conversion or submission failure reports the unsent
batch as handled, telling user space that invalidations which never made
it to the hardware have completed.
- The allocation is sized straight from the user's entry_num with no upper
bound and no __GFP_ACCOUNT, letting a large request drive an oversized,
unaccounted allocation and spam the page allocator.
- The -ENOMEM path returns without touching entry_num, once more reporting
the full input count as handled on a failure.
- A zero-length array, which the uAPI defines as a probe for a data_type
the kernel supports, reaches the full-array copy helper, which rejects
zero entries, so a supported type fails exactly like an unsupported one.
Rework the function to process the array one batch at a time, copying up to
CMDQ_BATCH_ENTRIES - 1 entries into a fixed-size stack buffer, converting
them in place, and issuing the batch, which removes the user-sized buffer.
A refactor is preferred over a series of smaller fixes because the problems
above are symptoms of one structure; changing the structure removes them as
a class rather than patching each in place:
- Dropping the allocation makes the size bound, the __GFP_ACCOUNT, and the
-ENOMEM accounting question moot, not three separate fixes.
- entry_num advances only as batches are issued, so the count it reports
tracks submitted commands and needs no correction on the error paths.
- The type is checked once up front, and an empty array then iterates zero
times, so the probe case falls out for free.
Each batch is still copied in one bulk transfer via the full-array helper,
iommu_copy_struct_from_full_user_array(); the whole-array copy becomes one
copy per batch that the preceding helper fix keeps to a single bulk copy.
If converting an entry fails, the already-converted prefix of the batch is
issued before the error is returned; if that issue fails, the batch is not
counted. So entry_num stays tied only to commands that actually reached the
hardware, without dropping the valid ones that preceded the failure.
Fixes: d68beb276ba2 ("iommu/arm-smmu-v3: Support IOMMU_HWPT_INVALIDATE using a
VIOMMU object")
Assisted-by: Codex:GPT-5
Signed-off-by: Nicolin Chen <[email protected]>
---
.../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 91 +++++++++++--------
1 file changed, 55 insertions(+), 36 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c
index ddae0b07c76b5..5574ccaecf381 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c
@@ -350,53 +350,72 @@ static int arm_vsmmu_convert_user_cmd(struct arm_vsmmu
*vsmmu,
return 0;
}
-int arm_vsmmu_cache_invalidate(struct iommufd_viommu *viommu,
- struct iommu_user_data_array *array)
+static int arm_vsmmu_cache_invalidate_batch(struct arm_vsmmu *vsmmu,
+ struct iommu_user_data_array *array,
+ u32 *idx)
{
- struct arm_vsmmu *vsmmu = container_of(viommu, struct arm_vsmmu, core);
+ struct arm_vsmmu_invalidation_cmd cmds[CMDQ_BATCH_ENTRIES - 1];
struct arm_smmu_device *smmu = vsmmu->smmu;
- struct arm_vsmmu_invalidation_cmd *last;
- struct arm_vsmmu_invalidation_cmd *cmds;
- struct arm_vsmmu_invalidation_cmd *cur;
- struct arm_vsmmu_invalidation_cmd *end;
- int ret;
-
- cmds = kzalloc_objs(*cmds, array->entry_num);
- if (!cmds)
- return -ENOMEM;
- cur = cmds;
- end = cmds + array->entry_num;
+ struct iommu_user_data_array batch = {
+ .type = array->type,
+ .entry_len = array->entry_len,
+ };
+ unsigned int num, i;
+ int ret, issue_ret;
static_assert(sizeof(*cmds) == 2 * sizeof(u64));
+
+ /* Copy one batch of the user array in a single bulk copy */
+ num = min_t(u32, array->entry_num - *idx, ARRAY_SIZE(cmds));
+ batch.uptr = array->uptr + array->entry_len * *idx;
+ batch.entry_num = num;
ret = iommu_copy_struct_from_full_user_array(
- cmds, sizeof(*cmds), array,
+ cmds, sizeof(*cmds), &batch,
IOMMU_VIOMMU_INVALIDATE_DATA_ARM_SMMUV3);
if (ret)
- goto out;
-
- last = cmds;
- while (cur != end) {
- ret = arm_vsmmu_convert_user_cmd(vsmmu, cur);
- if (ret)
- goto out;
-
- /* FIXME work in blocks of CMDQ_BATCH_ENTRIES and copy each
block? */
- cur++;
- if (cur != end && (cur - last) != CMDQ_BATCH_ENTRIES - 1)
- continue;
+ return ret;
- /* FIXME always uses the main cmdq rather than trying to group
by type */
- ret = arm_smmu_cmdq_issue_cmdlist(smmu, &smmu->cmdq, last->cmd,
- cur - last, true);
+ /* Convert in place; only the converted prefix may be issued */
+ for (i = 0; i < num; i++) {
+ ret = arm_vsmmu_convert_user_cmd(vsmmu, &cmds[i]);
if (ret) {
- cur--;
- goto out;
+ num = i;
+ break;
}
- last = cur;
}
-out:
- array->entry_num = cur - cmds;
- kfree(cmds);
+ if (!num)
+ return ret;
+
+ /* FIXME always uses the main cmdq rather than trying to group by type
*/
+ issue_ret = arm_smmu_cmdq_issue_cmdlist(smmu, &smmu->cmdq, cmds->cmd,
+ num, true);
+ if (issue_ret)
+ return issue_ret;
+
+ *idx += num;
+ return ret;
+}
+
+int arm_vsmmu_cache_invalidate(struct iommufd_viommu *viommu,
+ struct iommu_user_data_array *array)
+{
+ struct arm_vsmmu *vsmmu = container_of(viommu, struct arm_vsmmu, core);
+ u32 issued = 0;
+ int ret = 0;
+
+ if (array->type != IOMMU_VIOMMU_INVALIDATE_DATA_ARM_SMMUV3) {
+ array->entry_num = 0;
+ return -EINVAL;
+ }
+
+ while (issued != array->entry_num) {
+ /* Process and issue the command(s) in batch */
+ ret = arm_vsmmu_cache_invalidate_batch(vsmmu, array, &issued);
+ if (ret)
+ break;
+ }
+
+ array->entry_num = issued;
return ret;
}
--
2.43.0
