| Message ID | 20180730134321.19898-2-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | fix for bug #1783362 | expand |
Le 30/07/2018 à 15:43, Alex Bennée a écrit : > I've slightly re-organised the check to more closely match the > sequence that the kernel uses in do_mmap(). We check for both the zero > case (EINVAL) and the overflow length case (ENOMEM). > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Cc: umarcor <1783362@bugs.launchpad.net> > > --- > v2 > - add comment on overflow > --- > linux-user/mmap.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/linux-user/mmap.c b/linux-user/mmap.c > index d0c50e4888..41e0983ce8 100644 > --- a/linux-user/mmap.c > +++ b/linux-user/mmap.c > @@ -391,14 +391,23 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot, > } > #endif > > - if (offset & ~TARGET_PAGE_MASK) { > + if (!len) { > errno = EINVAL; > goto fail; > } > > + /* Also check for overflows... */ > len = TARGET_PAGE_ALIGN(len); > - if (len == 0) > - goto the_end; > + if (!len) { > + errno = ENOMEM; > + goto fail; > + } > + > + if (offset & ~TARGET_PAGE_MASK) { > + errno = EINVAL; > + goto fail; > + } > + > real_start = start & qemu_host_page_mask; > host_offset = offset & qemu_host_page_mask; > > Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Laurent Vivier <laurent@vivier.eu> writes: > Le 30/07/2018 à 15:43, Alex Bennée a écrit: >> I've slightly re-organised the check to more closely match the >> sequence that the kernel uses in do_mmap(). We check for both the zero >> case (EINVAL) and the overflow length case (ENOMEM). >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> Cc: umarcor <1783362@bugs.launchpad.net> >> >> --- >> v2 >> - add comment on overflow >> --- >> linux-user/mmap.c | 15 ++++++++++++--- >> 1 file changed, 12 insertions(+), 3 deletions(-) >> >> diff --git a/linux-user/mmap.c b/linux-user/mmap.c >> index d0c50e4888..41e0983ce8 100644 >> --- a/linux-user/mmap.c >> +++ b/linux-user/mmap.c >> @@ -391,14 +391,23 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot, >> } >> #endif >> >> - if (offset & ~TARGET_PAGE_MASK) { >> + if (!len) { >> errno = EINVAL; >> goto fail; >> } >> >> + /* Also check for overflows... */ >> len = TARGET_PAGE_ALIGN(len); >> - if (len == 0) >> - goto the_end; >> + if (!len) { >> + errno = ENOMEM; >> + goto fail; >> + } >> + >> + if (offset & ~TARGET_PAGE_MASK) { >> + errno = EINVAL; >> + goto fail; >> + } >> + >> real_start = start & qemu_host_page_mask; >> host_offset = offset & qemu_host_page_mask; >> >> > > Reviewed-by: Laurent Vivier <laurent@vivier.eu> Are you going to take this via your queue or do you want me to re-post with the r-b? -- Alex Bennée
Le 30/07/2018 à 16:21, Alex Bennée a écrit : > > Laurent Vivier <laurent@vivier.eu> writes: > >> Le 30/07/2018 à 15:43, Alex Bennée a écrit: >>> I've slightly re-organised the check to more closely match the >>> sequence that the kernel uses in do_mmap(). We check for both the zero >>> case (EINVAL) and the overflow length case (ENOMEM). >>> >>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>> Cc: umarcor <1783362@bugs.launchpad.net> >>> >>> --- >>> v2 >>> - add comment on overflow >>> --- >>> linux-user/mmap.c | 15 ++++++++++++--- >>> 1 file changed, 12 insertions(+), 3 deletions(-) >>> >>> diff --git a/linux-user/mmap.c b/linux-user/mmap.c >>> index d0c50e4888..41e0983ce8 100644 >>> --- a/linux-user/mmap.c >>> +++ b/linux-user/mmap.c >>> @@ -391,14 +391,23 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot, >>> } >>> #endif >>> >>> - if (offset & ~TARGET_PAGE_MASK) { >>> + if (!len) { >>> errno = EINVAL; >>> goto fail; >>> } >>> >>> + /* Also check for overflows... */ >>> len = TARGET_PAGE_ALIGN(len); >>> - if (len == 0) >>> - goto the_end; >>> + if (!len) { >>> + errno = ENOMEM; >>> + goto fail; >>> + } >>> + >>> + if (offset & ~TARGET_PAGE_MASK) { >>> + errno = EINVAL; >>> + goto fail; >>> + } >>> + >>> real_start = start & qemu_host_page_mask; >>> host_offset = offset & qemu_host_page_mask; >>> >>> >> >> Reviewed-by: Laurent Vivier <laurent@vivier.eu> > > Are you going to take this via your queue or do you want me to re-post > with the r-b? I can take this via my queue. Thanks, Laurent
On 07/30/2018 09:43 AM, Alex Bennée wrote: > I've slightly re-organised the check to more closely match the > sequence that the kernel uses in do_mmap(). We check for both the zero > case (EINVAL) and the overflow length case (ENOMEM). > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Cc: umarcor <1783362@bugs.launchpad.net> > > --- > v2 > - add comment on overflow > --- > linux-user/mmap.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/linux-user/mmap.c b/linux-user/mmap.c index d0c50e4888..41e0983ce8 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -391,14 +391,23 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot, } #endif - if (offset & ~TARGET_PAGE_MASK) { + if (!len) { errno = EINVAL; goto fail; } + /* Also check for overflows... */ len = TARGET_PAGE_ALIGN(len); - if (len == 0) - goto the_end; + if (!len) { + errno = ENOMEM; + goto fail; + } + + if (offset & ~TARGET_PAGE_MASK) { + errno = EINVAL; + goto fail; + } + real_start = start & qemu_host_page_mask; host_offset = offset & qemu_host_page_mask;
I've slightly re-organised the check to more closely match the sequence that the kernel uses in do_mmap(). We check for both the zero case (EINVAL) and the overflow length case (ENOMEM). Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Cc: umarcor <1783362@bugs.launchpad.net> --- v2 - add comment on overflow --- linux-user/mmap.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)