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.