exec: Fix non-power-of-2 sized accesses
PATCH v2 1/1 exec: Fix non-power-of-2 sized accesses
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.
---
v2: Remove unnecessary loop condition
exec.c | 7 +++++++ 1 file changed, 7 insertions(+)
exec.c +7 -0
@@ -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 ; + } + } +/* Don't attempt accesses larger than the maximum. */ if (l > access_size_max) { l = access_size_max;
Quoted 9 lines · Alex Williamson
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 <[email protected]>Reviewed-by Laszlo Ersek <[email protected]>
yeah
Quoted 6 lines
---
v2: Remove unnecessary loop condition
exec.c | 7 +++++++ 1 file changed, 7 insertions(+)
exec.c static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr) · line 1933
Quoted 7 lines · Alex Williamson
exec.c +7
-0+/* Size must be a power of 2 */ +if (l & (l -1 )) { +while (l & (access_size_max -1 )) { + access_size_max >>=1 ; + } + } +
Why the loop at all? x & -x extracts the lsb of x.
r~
Quoted 10 lines · Richard Henderson
exec.c static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr) · line 1933
Quoted 7 lines · Alex Williamson
exec.c +7
-0+/* Size must be a power of 2 */ +if (l & (l -1 )) { +while (l & (access_size_max -1 )) { + access_size_max >>=1 ; + } + } +Why the loop at all? x & -x extracts the lsb of x.
l = 5, we want 4, not 1. Thanks,
Alex
Quoted 10 lines · Alex Williamson
Quoted 9 lines · Richard Henderson
Quoted 7 lines · Alex Williamson
exec.c +7
-0+/* Size must be a power of 2 */ +if (l & (l -1 )) { +while (l & (access_size_max -1 )) { + access_size_max >>=1 ; + } + } +
Quoted 3 lines
Quoted 1 line
Why the loop at all? x & -x extracts the lsb of x.
l = 5, we want 4, not 1. Thanks,
Which is not what my patch does either :-\
Quoted 11 lines · Alex Williamson
Quoted 10 lines · Alex Williamson
Quoted 9 lines · Richard Henderson
Quoted 7 lines · Alex Williamson
exec.c +7
-0+/* Size must be a power of 2 */ +if (l & (l -1 )) { +while (l & (access_size_max -1 )) { + access_size_max >>=1 ; + } + } +
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Why the loop at all? x & -x extracts the lsb of x.
l = 5, we want 4, not 1. Thanks,
Which is not what my patch does either :-\
Simple change though
while (!(l & access_size_max))...
Quoted 10 lines · Alex Williamson
Quoted 9 lines · Richard Henderson
Quoted 7 lines · Alex Williamson
exec.c +7
-0+/* Size must be a power of 2 */ +if (l & (l -1 )) { +while (l & (access_size_max -1 )) { + access_size_max >>=1 ; + } + } +
Quoted 3 lines
Quoted 1 line
Why the loop at all? x & -x extracts the lsb of x.
l = 5, we want 4, not 1. Thanks,
It still doesn't require a loop.
l = 1 << (31 - clz32(l));
r~
Quoted 11 lines · Richard Henderson
Quoted 10 lines · Alex Williamson
Quoted 9 lines · Richard Henderson
Quoted 7 lines · Alex Williamson
exec.c +7
-0+/* Size must be a power of 2 */ +if (l & (l -1 )) { +while (l & (access_size_max -1 )) { + access_size_max >>=1 ; + } + } +
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Why the loop at all? x & -x extracts the lsb of x.
l = 5, we want 4, not 1. Thanks,
It still doesn't require a loop.
l = 1 << (31 - clz32(l));
or even pow2floor() declared in qemu-common.h and implemented in cutils.c. No need to reinvent what we already have.
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org