| Message ID | 20240815213231.303424-1-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [for-9.1] linux-user: Handle short reads in mmap_h_gt_g | expand |
在 2024/8/16 上午5:32, Richard Henderson 写道: > In particular, if an image has a large bss, we can hit > EOF before reading all host_len bytes of the mapping. > > Cc: qemu-stable@nongnu.org > Fixes: eb5027ac618 ("linux-user: Split out mmap_h_gt_g") > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2504 > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > linux-user/mmap.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) Reviewed-by: Song Gao <gaosong@loongson.cn> Tested-by: Song Gao <gaosong@loongson.cn> Thanks. Song Gao > diff --git a/linux-user/mmap.c b/linux-user/mmap.c > index 6418e811f6..de9ab13754 100644 > --- a/linux-user/mmap.c > +++ b/linux-user/mmap.c > @@ -853,10 +853,21 @@ static abi_long mmap_h_gt_g(abi_ulong start, abi_ulong len, > } > > if (misaligned_offset) { > - /* TODO: The read could be short. */ > - if (pread(fd, p, host_len, offset + real_start - start) != host_len) { > - do_munmap(p, host_len); > - return -1; > + size_t o = 0; > + while (1) { > + ssize_t r = pread(fd, p + o, host_len - o, > + o + offset + real_start - start); > + if (likely(r == host_len - o) || r == 0) { > + /* Complete or EOF */ > + break; > + } > + if (unlikely(r == -1)) { > + /* Error */ > + do_munmap(p, host_len); > + return -1; > + } > + /* Short read -- iterate */ > + o += r; > } > if (!(host_prot & PROT_WRITE)) { > mprotect(p, host_len, host_prot);
On 15/8/24 23:32, Richard Henderson wrote: > In particular, if an image has a large bss, we can hit > EOF before reading all host_len bytes of the mapping. > > Cc: qemu-stable@nongnu.org > Fixes: eb5027ac618 ("linux-user: Split out mmap_h_gt_g") > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2504 > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > linux-user/mmap.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/linux-user/mmap.c b/linux-user/mmap.c > index 6418e811f6..de9ab13754 100644 > --- a/linux-user/mmap.c > +++ b/linux-user/mmap.c > @@ -853,10 +853,21 @@ static abi_long mmap_h_gt_g(abi_ulong start, abi_ulong len, > } > > if (misaligned_offset) { > - /* TODO: The read could be short. */ I note there are other short reads in {linux,bsd}-user/mmap.c. Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > - if (pread(fd, p, host_len, offset + real_start - start) != host_len) { > - do_munmap(p, host_len); > - return -1; > + size_t o = 0; > + while (1) { > + ssize_t r = pread(fd, p + o, host_len - o, > + o + offset + real_start - start); > + if (likely(r == host_len - o) || r == 0) { > + /* Complete or EOF */ > + break; > + } > + if (unlikely(r == -1)) { > + /* Error */ > + do_munmap(p, host_len); > + return -1; > + } > + /* Short read -- iterate */ > + o += r; > } > if (!(host_prot & PROT_WRITE)) { > mprotect(p, host_len, host_prot);
On 8/16/24 15:57, Philippe Mathieu-Daudé wrote: > On 15/8/24 23:32, Richard Henderson wrote: >> In particular, if an image has a large bss, we can hit >> EOF before reading all host_len bytes of the mapping. >> >> Cc: qemu-stable@nongnu.org >> Fixes: eb5027ac618 ("linux-user: Split out mmap_h_gt_g") >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2504 >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> linux-user/mmap.c | 19 +++++++++++++++---- >> 1 file changed, 15 insertions(+), 4 deletions(-) >> >> diff --git a/linux-user/mmap.c b/linux-user/mmap.c >> index 6418e811f6..de9ab13754 100644 >> --- a/linux-user/mmap.c >> +++ b/linux-user/mmap.c >> @@ -853,10 +853,21 @@ static abi_long mmap_h_gt_g(abi_ulong start, abi_ulong len, >> } >> if (misaligned_offset) { >> - /* TODO: The read could be short. */ > > I note there are other short reads in {linux,bsd}-user/mmap.c. Ah, via mmap_frag. Hmm, yes. Worth fixing at the same time. r~
diff --git a/linux-user/mmap.c b/linux-user/mmap.c index 6418e811f6..de9ab13754 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -853,10 +853,21 @@ static abi_long mmap_h_gt_g(abi_ulong start, abi_ulong len, } if (misaligned_offset) { - /* TODO: The read could be short. */ - if (pread(fd, p, host_len, offset + real_start - start) != host_len) { - do_munmap(p, host_len); - return -1; + size_t o = 0; + while (1) { + ssize_t r = pread(fd, p + o, host_len - o, + o + offset + real_start - start); + if (likely(r == host_len - o) || r == 0) { + /* Complete or EOF */ + break; + } + if (unlikely(r == -1)) { + /* Error */ + do_munmap(p, host_len); + return -1; + } + /* Short read -- iterate */ + o += r; } if (!(host_prot & PROT_WRITE)) { mprotect(p, host_len, host_prot);
In particular, if an image has a large bss, we can hit EOF before reading all host_len bytes of the mapping. Cc: qemu-stable@nongnu.org Fixes: eb5027ac618 ("linux-user: Split out mmap_h_gt_g") Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2504 Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- linux-user/mmap.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)