| Message ID | 1321873447-15318-1-git-send-email-agraf@suse.de |
|---|---|
| State | New |
| Headers | show |
On 21 November 2011 11:04, Alexander Graf <agraf@suse.de> wrote: > static void > print_syscall_ret_addr(const struct syscallname *name, abi_long ret) > { > -if( ret == -1 ) { > - gemu_log(" = -1 errno=%d (%s)\n", errno, target_strerror(errno)); > + char *errstr = NULL; > + > + if (ret == -1) { > + errstr = target_strerror(errno); > + } > + if ((ret == -1) && errstr) { > + gemu_log(" = -1 errno=%d (%s)\n", errno, errstr); > } else { > gemu_log(" = 0x" TARGET_ABI_FMT_lx "\n", ret); > } Looking more closely at this function I'm pretty sure it's wrong. We shouldn't be calling target_strerror() on errno, because the syscall.c code has already put the errno into ret as -thing. This code should look exactly like the default case in print_syscall_ret() except that the format string used should be TARGET_ABI_FMT_lx rather than TARGET_ABI_FMT_ld. You can see the problem if you compile and run this test program: #include <sys/mman.h> int main(void) { mmap(0, 0xc0000000, PROT_READ, MAP_ANONYMOUS, -1, 0); return 0; } Run under native strace it prints: mmap2(NULL, 3221225472, PROT_READ, MAP_FILE|MAP_ANONYMOUS, -1, 0) = -1 ENOMEM (Cannot allocate memory) Run under i386-linux-user/qemu-i386 -strace it prints: 5892 mmap2(NULL,-1073741824,PROT_READ,MAP_ANONYMOUS,-1,0) = 0xfffffff4 because we haven't noticed that that return value is an errno. -- PMM
diff --git a/linux-user/strace.c b/linux-user/strace.c index 90027a1..269481e 100644 --- a/linux-user/strace.c +++ b/linux-user/strace.c @@ -284,8 +284,13 @@ print_ipc(const struct syscallname *name, static void print_syscall_ret_addr(const struct syscallname *name, abi_long ret) { -if( ret == -1 ) { - gemu_log(" = -1 errno=%d (%s)\n", errno, target_strerror(errno)); + char *errstr = NULL; + + if (ret == -1) { + errstr = target_strerror(errno); + } + if ((ret == -1) && errstr) { + gemu_log(" = -1 errno=%d (%s)\n", errno, errstr); } else { gemu_log(" = 0x" TARGET_ABI_FMT_lx "\n", ret); } @@ -1515,14 +1520,19 @@ void print_syscall_ret(int num, abi_long ret) { int i; + char *errstr = NULL; for(i=0;i<nsyscalls;i++) if( scnames[i].nr == num ) { if( scnames[i].result != NULL ) { scnames[i].result(&scnames[i],ret); } else { - if( ret < 0 ) { - gemu_log(" = -1 errno=" TARGET_ABI_FMT_ld " (%s)\n", -ret, target_strerror(-ret)); + if (ret < 0) { + errstr = target_strerror(-ret); + } + if (errstr) { + gemu_log(" = -1 errno=" TARGET_ABI_FMT_ld " (%s)\n", + -ret, errstr); } else { gemu_log(" = " TARGET_ABI_FMT_ld "\n", ret); } diff --git a/linux-user/syscall.c b/linux-user/syscall.c index f227097..f170724 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -731,6 +731,9 @@ static inline int is_error(abi_long ret) char *target_strerror(int err) { + if ((err >= ERRNO_TABLE_SIZE) || (err < 0)) { + return NULL; + } return strerror(target_to_host_errno(err)); }
While debugging some issues with QEMU_STRACE I stumbled over segmentation faults that were pretty reproducible. Turns out we tried to treat a normal return value as errno, resulting in an access over array boundaries for the resolution. Fix this by allowing failure to resolve invalid errnos into strings. Signed-off-by: Alexander Graf <agraf@suse.de> --- v1 -> v2: - propagate fault further down, so we display the negative value v2 -> v3: - fix boolean logic - fix print_syscall_ret_addr --- linux-user/strace.c | 18 ++++++++++++++---- linux-user/syscall.c | 3 +++ 2 files changed, 17 insertions(+), 4 deletions(-)