From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42545) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VAiHj-0000Zh-Pt for qemu-devel@nongnu.org; Sat, 17 Aug 2013 11:19:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VAiHf-0003OE-8B for qemu-devel@nongnu.org; Sat, 17 Aug 2013 11:19:23 -0400 Message-ID: <1376752757.28796.68.camel@ul30vt.home> From: Alex Williamson Date: Sat, 17 Aug 2013 09:19:17 -0600 In-Reply-To: <520F194F.1030501@redhat.com> References: <20130816215706.23647.80992.stgit@bling.home> <520F194F.1030501@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4] exec: Fix non-power-of-2 sized accesses List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: rth@twiddle.net, lersek@redhat.com, qemu-devel@nongnu.org, qemu-stable@nongnu.org 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 > > Cc: qemu-stable@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, > > > > > > >