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 <[email protected]> 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
