All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Mastro <amastro@fb.com>
To: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Alex Williamson <alex.williamson@redhat.com>,
	Alejandro Jimenez <alejandro.j.jimenez@oracle.com>,
	<kvm@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 1/3] vfio/type1: sanitize for overflow using check_*_overflow
Date: Wed, 8 Oct 2025 15:19:07 -0700	[thread overview]
Message-ID: <aObjW9VxYMkFQ1KB@devgpu015.cco6.facebook.com> (raw)
In-Reply-To: <aOaFqZ5cPgeRyoNS@devgpu015.cco6.facebook.com>

On Wed, Oct 08, 2025 at 08:39:21AM -0700, Alex Mastro wrote:
> On Wed, Oct 08, 2025 at 09:19:30AM -0300, Jason Gunthorpe wrote:
> > On Tue, Oct 07, 2025 at 09:08:46PM -0700, Alex Mastro wrote:
> > > +	if (check_add_overflow(user_iova, iova_size - 1, &iova_end))
> > > +		return -EINVAL;
> > 
> > Let's be consistent with iommufd/etc, 'end' is start+size 'last' is start+size-1
> > 
> > Otherwise it is super confusing :(
> 
> 
> Both suggestions SGTM.

I'm not sure about the latter anymore. There's somewhat pervasive precedent for
using 'end' as the inclusive limit in vfio_iommu_type1.c. I am all for making
things less confusing. I don't think I can introduce 'end' 'last' convention
without preparing the existing code first.

Thoughts? Spend another commit renaming this to 'last'? Tolerate inconsistency
between vfio and iommufd?

116  struct vfio_iova {
117          struct list_head        list;
118          dma_addr_t              start;
119          dma_addr_t              end;
120  };
...
2037                  end = resv->start + resv->length - 1;
2038
2039                  list_for_each_entry_safe(n, next, iova, list) {
2040                          int ret = 0;
2041
2042                          /* No overlap */
2043                          if (start > n->end || end < n->start)
2044                                  continue;
...
2052                          if (start > n->start)
2053                                  ret = vfio_iommu_iova_insert(&n->list, n->start,
2054                                                               start - 1);
2055                          if (!ret && end < n->end)
2056                                  ret = vfio_iommu_iova_insert(&n->list, end + 1,
2057                                                               n->end);

  reply	other threads:[~2025-10-08 22:19 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-08  4:08 [PATCH v2 0/3] vfio: handle DMA map/unmap up to the addressable limit Alex Mastro
2025-10-08  4:08 ` [PATCH v2 1/3] vfio/type1: sanitize for overflow using check_*_overflow Alex Mastro
2025-10-08 12:19   ` Jason Gunthorpe
2025-10-08 15:39     ` Alex Mastro
2025-10-08 22:19       ` Alex Mastro [this message]
2025-10-09  1:15         ` Jason Gunthorpe
2025-10-09 18:01           ` Alex Mastro
2025-10-08  4:08 ` [PATCH v2 2/3] vfio/type1: move iova increment to unmap_unpin_* caller Alex Mastro
2025-10-08  4:08 ` [PATCH v2 3/3] vfio/type1: handle DMA map/unmap up to the addressable limit Alex Mastro
2025-10-09  0:25   ` Alex Mastro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aObjW9VxYMkFQ1KB@devgpu015.cco6.facebook.com \
    --to=amastro@fb.com \
    --cc=alejandro.j.jimenez@oracle.com \
    --cc=alex.williamson@redhat.com \
    --cc=jgg@ziepe.ca \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.