> -----Original Message----- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 08 August 2018 11:47 > To: Andrew Cooper <andrew.coop...@citrix.com>; Paul Durrant > <paul.durr...@citrix.com> > Cc: Wei Liu <wei.l...@citrix.com>; George Dunlap > <george.dun...@citrix.com>; Ian Jackson <ian.jack...@citrix.com>; > Stefano Stabellini <sstabell...@kernel.org>; xen-devel <xen- > de...@lists.xenproject.org>; Konrad Rzeszutek Wilk > <konrad.w...@oracle.com>; Tim (Xen.org) <t...@xen.org> > Subject: Re: [PATCH v21 1/2] common: add a new mappable resource type: > XENMEM_resource_grant_table > > >>> On 08.08.18 at 12:10, <andrew.coop...@citrix.com> wrote: > > On 08/08/18 10:00, Paul Durrant wrote: > >> +static int gnttab_get_status_frame_mfn(struct domain *d, > >> + unsigned long idx, mfn_t *mfn) > >> +{ > >> + struct grant_table *gt = d->grant_table; > >> + > >> + ASSERT(gt->gt_version == 2); > >> + > >> + if ( idx >= nr_status_frames(gt) ) > >> + { > >> + unsigned long nr = status_to_grant_frames(idx + 1); > > > > Why the +1 ? Won't that cause a failure if you attempt to map the > > maximum valid index? > > That's the normal index-of-something to count-of-something > conversion (or else the table may get grown by too little). I've > instead been considering the badness of overflow here, but > decided to leave it uncommented as the check further down > would at least not make this insecure. However, with ... > > >> + > >> + if ( nr <= gt->max_grant_frames ) > >> + gnttab_grow_table(d, nr); > > > > You want to capture the return value of grow_table(), which at least > > distinguishes between ENODEV and ENOMEM. > > > >> + > >> + if ( idx >= nr_status_frames(gt) ) > >> + return -EINVAL; > > > > This can probably(?) be asserted if gnttab_grow_table() returns > > successfully. > > ... these two a potential overflow above would then have a > chance of triggering the assertion you suggest to add. > > As to the grow_table() return value check - I'd prefer if the > patch here didn't alter original behavior. If we want it altered, > better in a separate patch.
Ok. I'll leave the return value of gnttab_grow_table() unchecked as-is. > > Jan > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
