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

[Robin Murphy <robin.murphy@arm.com>]

On 2026-06-05 9:59 am, Maoyi Xie wrote:
> Hi Jean-Philippe,
> 
> On Fri, Jun 5, 2026 at 12:04 AM Jean-Philippe Brucker <jpb@kernel.org> 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.

If that were a real issue, then I think list_entry_is_head() in the 
list_for_each_entry() iteration itself would suffer from it too. It's 
probably safe to say that while the C language in general leaves this 
open, Linux relies on GCC using pointer representations where the 
arithmetic will always work out, same as we depend on two's complement 
representation of integers all over the place.

I believe the concern is more just that if the pattern of using a 
list_entry iterator variable outside the scope of the loop isn't 
discouraged, then it's all too easy for subsequent code to inadvertently 
dereference fields *other* than the list_head without checking that it 
is a valid entry, which definitely would then be the worst kind of UB.

Thanks,
Robin.

> 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.
> 
> Thanks,
> Maoyi