On Wed, Aug 19, 2026 at 12:36 AM Akihiko Odaki
<[email protected]> wrote:
>
> > +/* Memory region shared with back-end when memory-isolation is active */
> > +typedef struct {
> > +    void *shared_mem_addr; /* mapped shared memory */
> > +    VhostIOVATree *tree; /* controls mapping of regions into IOVA space */
> > +    void *vring_hva_addr; /* beginning of vring region in shared memory */
> > +    size_t vring_region_size; /* amount of shared memory reserved for 
> > vrings */
> > +    size_t size; /* size of the mapped shared memory */
> > +    int fd; /* descriptor of anonymous file backing shared iso region */
> > +    Int128 iso_iova_offset; /* translation from IOVA to hva of iso region 
> > */
>
> This Int128 usage is misleading. The value derived from iso_iova_offset
> is always consumed with int128_get64(), which asserts the value fits in
> 64-bit. But we actually need to enforce that the address before
> translation resides in the shared memory. So making iso_iova_offset
> Int128 is not a good way to assert the real invariant.
>
> Furthermore, "[PATCH RFC v2 10/13] hw/virtio/vhost-user: handle data
> movement with shadow vqs" has:
>
>      ptrdiff_t offset = u->iso_mem_ctx.iso_iova_offset;
>
> If defined(CONFIG_INT128) && !defined(CONFIG_TCG_INTERPRETER), this will
> overflow with u->iso_mem_ctx.iso_iova_offset > PTRDIFF_MAX. Otherwise,
> Int128 will be a struct, which causes a compilation error.
>
> uintptr_t is sufficient if the invariant is enforced in some alternative
> way. A negative offset would be represented as wraparound, and a
> positive one will never result in wraparound. Just always ensure the
> address before translation resides in the shared memory.
>
> Regards,
> Akihiko Odaki
>

Your point is taken on the Int128!  I also got similar feedback from
Stefan Hajnoczi and Hanna CzenCzek that it was unnecessary
complication.  Will move it to uintptr_t and double check the
bounds-checking on translations in the later patches to confirm that
it didn't depend on the Int128 type.

Also, I thought I had moved all cases of ptrdiff_t to Int128, but I
apparently missed that one.  The offset must have been less than
PTRDIFF_MAX in testing since this didn't come up.  Will fix and change
to uintptr_t as well.

Thank you once again!
Connor

Reply via email to