Re: [PATCH 2/2] iommu/virtio: Avoid using the list iterator past the loop in viommu_add_resv_mem()

[Jean-Philippe Brucker]

On Fri, Jun 05, 2026 at 04:59:47PM +0800, Maoyi Xie wrote:
> Hi Jean-Philippe,
> 
> On Fri, Jun 5, 2026 at 12:04 AM Jean-Philippe Brucker <jpb@xxxxxxxxxx> wrote:
> > Thank you for removing this hack, though I don't find a contract in the
> > list_for_each_entry() doc, and the fix still accesses a cursor outside the
> > loop. Since you mentioned C11 UB in another email, do you have more info
> > on the precise operation which is undefined in the kernel (container_of
> > into an invalid object or the &next->list addition)?  Just so I can avoid
> > it in the future.
> 
> > Anyway, thanks for the patch. If this is just general cleanup that's fine
> > too.
> 
> Thanks for the review. You're right.
> 
> There is no such contract in the docs. I overstated it. And no undefined
> pointer arithmetic happens here either.
> 
> iommu_resv_region has list as its first member, so offsetof is 0. That
> makes container_of(&vdev->resv_regions, struct iommu_resv_region, list)
> just (char *)&vdev->resv_regions - 0, i.e. the head with a different
> type. &next->list is the head again. Nothing reads next as an
> iommu_resv_region, so no pointer leaves its object.
> 
> The container_of would be undefined only if list was not the first
> member. Then the subtraction lands before the real object (C11 6.5.6).
> That is not the case here.
> 
> So this is cleanup, not a UB fix. The typed cursor points at the head but
> reads like an entry. That becomes a real bug the day someone reorders the
> struct or touches another field. The new pos stays a struct list_head *,
> which is just the head, so it avoids all of that. I should have said that
> in the message.
> 
> If you or Joerg prefer, I can resend the series with the wording fixed.

For me the patch is fine as is, I was just curious about what the kernel
expects for pointer arithmetic. Thanks for clarifying

Thanks,
Jean