On Mon, Jun 01, 2026 at 01:42:37PM -0700, Nicolin Chen wrote:
> The cookie returned by xa_alloc() in iommufd_fault_fops_read() is per fault
> group, but the inner copy_to_user() runs per fault inside the group. If a
> copy fails mid-group, xa_erase clears the cookie and the group is restored
> to the deliver list, yet done is not rolled back. The function returns the
> partial byte count, with the successfully copied faults sitting at offsets
> below done carrying the now-erased cookie. The next read() then re-fetches
> the group, allocates a fresh cookie, and re-delivers every fault including
> the ones already copied; userspace sees duplicates carrying the new cookie,
> and a stale cookie that can never be responded to.
>
> Use a local group_done variable that tracks the per-group progress inside
> the inner loop, and only commit done = group_done after the inner loop has
> finished successfully. On a copy_to_user failure the outer break skips the
> commit, so done remains at its prior start-of-group baseline; the partial
> bytes already written past done are undefined to userspace per the read(2)
> contract, and the next read re-delivers the whole group atomically.
>
> Fixes: 07838f7fd529 ("iommufd: Add iommufd fault object")
> Cc: [email protected]
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Nicolin Chen <[email protected]>
Reviewed-by: Pranjal Shrivastava <[email protected]>
Thanks,
Praan
