On Sun, 2013-08-25 at 17:41 +0100, Alexander Graf wrote:
> > +    vcap = &req->iu.mad.capabilities;
> > +    rc = spapr_vio_dma_read(&s->vdev, be64_to_cpu(vcap->buffer),
> > +                            &cap,
> be16_to_cpu(vcap->common.length));
> 
> While I don't think any harm could happen from it, this could lead to
> a potential timing attack where we read and write from different
> locations in memory if the guest swizzles the request while we're
> processing it.

BTW. While I disagree with your initial comment ... is there any bound
checking here ? That looks like potential stack corruption unless I
miss something if the guest passes a too big length...

So at least the length should be read once, bound-checked, then the read
done with the result (don't bound check and read again, that would be
indeed racy).

Cheers,
Ben.



Reply via email to