On Tue, Sep 29, 2020 at 09:44:48PM +0200, Auger Eric wrote:
> Hi Stefan,
>
> On 9/29/20 5:59 PM, Stefan Hajnoczi wrote:
> > On Tue, Sep 29, 2020 at 10:55:50AM +0200, Eric Auger wrote:
> >> diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
> >> index ba0ee6e21c..71145970f3 100644
> >> --- a/util/vfio-helpers.c
> >> +++ b/util/vfio-helpers.c
> >> @@ -667,6 +667,50 @@ static bool qemu_vfio_verify_mappings(QEMUVFIOState
> >> *s)
> >> return true;
> >> }
> >>
> >> +static int
> >> +qemu_vfio_find_fixed_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
> >> +{
> >> + int i;
> >> +
> >> + for (i = 0; i < s->nb_iova_ranges; i++) {
> >> + if (s->usable_iova_ranges[i].end < s->low_water_mark) {
> >> + continue;
> >> + }
> >> + s->low_water_mark =
> >> + MAX(s->low_water_mark, s->usable_iova_ranges[i].start);
> >> +
> >> + if (s->usable_iova_ranges[i].end - s->low_water_mark + 1 >= size
> >> ||
> >> + s->usable_iova_ranges[i].end - s->low_water_mark + 1 == 0) {
> >
> > I don't understand the == 0 case. It seems like we are allocating an
> > IOVA beyond usable_iova_ranges[i].end?>
> It is meant to handle the case were low_water_mark = 0 and
> s->usable_iova_ranges[0].end = ULLONG_MAX (I know it cannot exist at the
> moment but may happen in the future) where we get an overflow. Given the
> if (s->usable_iova_ranges[i].end < s->low_water_mark) {
> continue;
> }
> I think this prevents us from allocating beyond
> usable_iova_ranges[i].end or do I miss something?Yes, you are right. Here are the constraints: e >= l j = max(l, s) e - j + 1 < s e - j + 1 == 0 Assume l >= s so we can replace j with l: e >= l e - l + 1 < s e - l + 1 == 0 The case I'm worried about is when the iova range cannot fit s bytes. The last condition is only true when e = l - 1, but this violates the first condition e >= l. So the problem scenario cannot occur. Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com>
signature.asc
Description: PGP signature
