| Message ID | 20230630132159.376995-10-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | linux-user: mmap range fixes | expand |
Richard Henderson <richard.henderson@linaro.org> writes: > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > linux-user/mmap.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/linux-user/mmap.c b/linux-user/mmap.c > index cb5369d2d1..41c2f09fd5 100644 > --- a/linux-user/mmap.c > +++ b/linux-user/mmap.c > @@ -502,7 +502,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot, > * If the user is asking for the kernel to find a location, do that > * before we truncate the length for mapping files below. > */ > - if (!(flags & MAP_FIXED)) { > + if (!(flags & (MAP_FIXED | MAP_FIXED_NOREPLACE))) { > host_len = len + offset - host_offset; > host_len = HOST_PAGE_ALIGN(host_len); > start = mmap_find_vma(real_start, host_len, TARGET_PAGE_SIZE); > @@ -544,7 +544,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot, > } > } > > - if (!(flags & MAP_FIXED)) { > + if (!(flags & (MAP_FIXED | MAP_FIXED_NOREPLACE))) { > unsigned long host_start; > void *p; > > @@ -593,6 +593,13 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot, > goto fail; > } > > + /* Validate that the chosen range is empty. */ > + if ((flags & MAP_FIXED_NOREPLACE) > + && !page_check_range_empty(start, end - 1)) { > + errno = EEXIST; > + goto fail; > + } > + > /* > * worst case: we cannot map the file because the offset is not > * aligned, so we read it > @@ -608,7 +615,8 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot, > goto fail; > } > retaddr = target_mmap(start, len, target_prot | PROT_WRITE, > - MAP_FIXED | MAP_PRIVATE | MAP_ANONYMOUS, > + (flags & (MAP_FIXED | MAP_FIXED_NOREPLACE)) > + | MAP_PRIVATE | MAP_ANONYMOUS, I thought we always MAP_FIXED because we've already jumped the hoops to work out where in the memory space this allocation is going. Now if the guest doesn't specify MAP_FIXED the kernel might decide to put the memory somewhere else, potentially out of reach of the guest addressing? IOW I thought the MAP_FIXED here was an internal QEMU implementation details rather than reflecting the guests wishes. > -1, 0); > if (retaddr == -1) { > goto fail;
On 7/3/23 11:51, Alex Bennée wrote: >> @@ -544,7 +544,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot, >> } >> } >> >> - if (!(flags & MAP_FIXED)) { >> + if (!(flags & (MAP_FIXED | MAP_FIXED_NOREPLACE))) { >> unsigned long host_start; >> void *p; >> ... >> @@ -608,7 +615,8 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot, >> goto fail; >> } >> retaddr = target_mmap(start, len, target_prot | PROT_WRITE, >> - MAP_FIXED | MAP_PRIVATE | MAP_ANONYMOUS, >> + (flags & (MAP_FIXED | MAP_FIXED_NOREPLACE)) >> + | MAP_PRIVATE | MAP_ANONYMOUS, > > I thought we always MAP_FIXED because we've already jumped the hoops to > work out where in the memory space this allocation is going. We have not, no. Not here, anyway. > IOW I thought the MAP_FIXED here was an internal QEMU implementation > details rather than reflecting the guests wishes. As I read it, MAP_FIXED was here because we'd already checked for it in the IF that I retained above. But now we're checking for 2 things, and must pass that on to the recursion. r~
diff --git a/linux-user/mmap.c b/linux-user/mmap.c index cb5369d2d1..41c2f09fd5 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -502,7 +502,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot, * If the user is asking for the kernel to find a location, do that * before we truncate the length for mapping files below. */ - if (!(flags & MAP_FIXED)) { + if (!(flags & (MAP_FIXED | MAP_FIXED_NOREPLACE))) { host_len = len + offset - host_offset; host_len = HOST_PAGE_ALIGN(host_len); start = mmap_find_vma(real_start, host_len, TARGET_PAGE_SIZE); @@ -544,7 +544,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot, } } - if (!(flags & MAP_FIXED)) { + if (!(flags & (MAP_FIXED | MAP_FIXED_NOREPLACE))) { unsigned long host_start; void *p; @@ -593,6 +593,13 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot, goto fail; } + /* Validate that the chosen range is empty. */ + if ((flags & MAP_FIXED_NOREPLACE) + && !page_check_range_empty(start, end - 1)) { + errno = EEXIST; + goto fail; + } + /* * worst case: we cannot map the file because the offset is not * aligned, so we read it @@ -608,7 +615,8 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot, goto fail; } retaddr = target_mmap(start, len, target_prot | PROT_WRITE, - MAP_FIXED | MAP_PRIVATE | MAP_ANONYMOUS, + (flags & (MAP_FIXED | MAP_FIXED_NOREPLACE)) + | MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); if (retaddr == -1) { goto fail;
Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- linux-user/mmap.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)