On Sat, 2013-08-17 at 08:33 +0200, Paolo Bonzini wrote:
> Il 16/08/2013 23:58, Alex Williamson ha scritto:
> > Since commit 23326164 we align access sizes to match the alignment of
> > the address, but we don't align the access size itself. This means we
> > let illegal access sizes (ex. 3) slip through if the address is
> > sufficiently aligned (ex. 4). This results in an abort which would be
> > easy for a guest to trigger. Account for aligning the access size.
>
> Is it the same as this?
>
> http://lists.gnu.org/archive/html/qemu-devel/2013-07/msg05398.html
>
> (which perhaps is buggy as your v1/v2/v3 :))?
Too bad this didn't make 1.6. I suspect your patch is ok because I
don't think we're going to see it called with a length greater than 8.
Maybe I don't even need that test in my version, but it's reassuring to
have it. As I note in my reply to Laszlo, using generic power-of-2
functions is quite a bit slower than the limited case we need to handle,
so while initially tempted by fancy algorithms, I actually prefer the
version below. Thanks,
Alex
> > Signed-off-by: Alex Williamson <alex.william...@redhat.com>
> > Cc: qemu-sta...@nongnu.org
> > ---
> >
> > v4: KISS
> > v3: Highest power of 2, not lowest
> > v2: Remove unnecessary loop condition
> >
> > exec.c | 18 +++++++++++++-----
> > 1 file changed, 13 insertions(+), 5 deletions(-)
> >
> > diff --git a/exec.c b/exec.c
> > index 3ca9381..67a822c 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -1924,12 +1924,20 @@ static int memory_access_size(MemoryRegion *mr,
> > unsigned l, hwaddr addr)
> > }
> > }
> >
> > - /* Don't attempt accesses larger than the maximum. */
> > - if (l > access_size_max) {
> > - l = access_size_max;
> > + /* Don't attempt accesses larger than the maximum or unsupported
> > sizes. */
> > + if (l >= access_size_max) {
> > + return access_size_max;
> > + } else {
> > + if (l >= 8) {
> > + return 8;
> > + } else if (l >= 4) {
> > + return 4;
> > + } else if (l >= 2) {
> > + return 2;
> > + } else {
> > + return 1;
> > + }
> > }
> > -
> > - return l;
> > }
> >
> > bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
> >
> >
> >
>
