On Fri, 2013-08-16 at 09:10 +0200, Laszlo Ersek wrote:
> On 08/16/13 06:55, Alex Williamson wrote:
> > 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.
> >
> > Signed-off-by: Alex Williamson <alex.william...@redhat.com>
> > Cc: qemu-sta...@nongnu.org
> > ---
> >
> > In the example I saw the guest was doing a 4-byte read at I/O port
> > 0xcd7. We satisfy the first byte with a 1-byte read leaving 3 bytes
> > remaining at an 8-byte aligned address... boom. ffs() caused weird
> > stack smashing errors here, so I just did a loop since it can only
> > run for a few iterations max.
> >
> > exec.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/exec.c b/exec.c
> > index 3ca9381..652fc3a 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -1924,6 +1924,13 @@ static int memory_access_size(MemoryRegion *mr,
> > unsigned l, hwaddr addr)
> > }
> > }
> >
> > + /* Size must be a power of 2 */
> > + if (l & (l - 1)) {
> > + while (l & (access_size_max - 1) && access_size_max > 1) {
> > + access_size_max >>= 1;
> > + }
> > + }
> > +
> > /* Don't attempt accesses larger than the maximum. */
> > if (l > access_size_max) {
> > l = access_size_max;
> >
> >
>
> Assuming that "access_size_max" is positive when reaching the code
> you're adding (and it does seem positive at that point), you don't need
> "&& access_size_max > 1". That expression won't be evaluated when it
> would matter (ie. when access_size_max==1).
>
> Anyway that's not a bug.
>
> Reviewed-by: Laszlo Ersek <ler...@redhat.com>
I realized this after I went to bed too. I'll send a v2 w/o the second
condition. Thanks,
Alex
