On Mon, Oct 17, 2016 at 04:24:24PM +0300, riku.voi...@linaro.org wrote: > From: Aleksandar Markovic <aleksandar.marko...@imgtec.com> > > There are currently several problems related to syslog() support. > > For example, if the second argument "bufp" of target syslog() syscall > is NULL, the current implementation always returns error code EFAULT. > However, NULL is a perfectly valid value for the second argument for > many use cases of this syscall. This is, for example, visible from > this excerpt of man page for syslog(2): > > > EINVAL Bad arguments (e.g., bad type; or for type 2, 3, or 4, buf is > > NULL, or len is less than zero; or for type 8, the level is > > outside the range 1 to 8). > > Moreover, the argument "bufp" is ignored for all cases of values of the > first argument, except 2, 3 and 4. This means that for such cases > (the first argument is not 2, 3 or 4), there is no need to pass "buf" > between host and target, and it can be set to NULL while calling host's > syslog(), without loss of emulation accuracy. > > Note also that if "bufp" is NULL and the first argument is 2, 3 or 4, the > correct returned error code is EINVAL, not EFAULT. > > All these details are reflected in this patch. > > "#ifdef TARGET_NR_syslog" is also proprerly inserted when needed. > > Support for Qemu's "-strace" switch for syslog() syscall is included too. > > LTP tests syslog11 and syslog12 pass with this patch (while fail without > it), on any platform. > > Signed-off-by: Aleksandar Markovic <aleksandar.marko...@imgtec.com> > Signed-off-by: Riku Voipio <riku.voi...@linaro.org> > --- > linux-user/strace.c | 72 > +++++++++++++++++++++++++++++++++++++++++++++++ > linux-user/strace.list | 2 +- > linux-user/syscall.c | 49 ++++++++++++++++++++++++++++---- > linux-user/syscall_defs.h | 25 ++++++++++++++++ > 4 files changed, 141 insertions(+), 7 deletions(-) > > diff --git a/linux-user/strace.c b/linux-user/strace.c > index a0e45b5..679f840 100644 > --- a/linux-user/strace.c > +++ b/linux-user/strace.c > @@ -1827,6 +1827,78 @@ print_rt_sigprocmask(const struct syscallname *name, > } > #endif > > +#ifdef TARGET_NR_syslog > +static void > +print_syslog_action(abi_ulong arg, int last) > +{ > + const char *type; > + > + switch (arg) { > + case TARGET_SYSLOG_ACTION_CLOSE: { > + type = "SYSLOG_ACTION_CLOSE"; > + break; > + } > + case TARGET_SYSLOG_ACTION_OPEN: { > + type = "SYSLOG_ACTION_OPEN"; > + break; > + } > + case TARGET_SYSLOG_ACTION_READ: { > + type = "SYSLOG_ACTION_READ"; > + break; > + } > + case TARGET_SYSLOG_ACTION_READ_ALL: { > + type = "SYSLOG_ACTION_READ_ALL"; > + break; > + } > + case TARGET_SYSLOG_ACTION_READ_CLEAR: { > + type = "SYSLOG_ACTION_READ_CLEAR"; > + break; > + } > + case TARGET_SYSLOG_ACTION_CLEAR: { > + type = "SYSLOG_ACTION_CLEAR"; > + break; > + } > + case TARGET_SYSLOG_ACTION_CONSOLE_OFF: { > + type = "SYSLOG_ACTION_CONSOLE_OFF"; > + break; > + } > + case TARGET_SYSLOG_ACTION_CONSOLE_ON: { > + type = "SYSLOG_ACTION_CONSOLE_ON"; > + break; > + } > + case TARGET_SYSLOG_ACTION_CONSOLE_LEVEL: { > + type = "SYSLOG_ACTION_CONSOLE_LEVEL"; > + break; > + } > + case TARGET_SYSLOG_ACTION_SIZE_UNREAD: { > + type = "SYSLOG_ACTION_SIZE_UNREAD"; > + break; > + } > + case TARGET_SYSLOG_ACTION_SIZE_BUFFER: { > + type = "SYSLOG_ACTION_SIZE_BUFFER"; > + break; > + } > + default: { > + print_raw_param("%ld", arg, last); > + return; > + } > + } > + gemu_log("%s%s", type, get_comma(last)); > +} > + > +static void > +print_syslog(const struct syscallname *name, > + abi_long arg0, abi_long arg1, abi_long arg2, > + abi_long arg3, abi_long arg4, abi_long arg5) > +{ > + print_syscall_prologue(name); > + print_syslog_action(arg0, 0); > + print_pointer(arg1, 0); > + print_raw_param("%d", arg2, 1); > + print_syscall_epilogue(name); > +} > +#endif > + > #ifdef TARGET_NR_mknod > static void > print_mknod(const struct syscallname *name, > diff --git a/linux-user/strace.list b/linux-user/strace.list > index f6dd044..2c7ad2b 100644 > --- a/linux-user/strace.list > +++ b/linux-user/strace.list > @@ -1486,7 +1486,7 @@ > { TARGET_NR_sys_kexec_load, "sys_kexec_load" , NULL, NULL, NULL }, > #endif > #ifdef TARGET_NR_syslog > -{ TARGET_NR_syslog, "syslog" , NULL, NULL, NULL }, > +{ TARGET_NR_syslog, "syslog" , NULL, print_syslog, NULL }, > #endif > #ifdef TARGET_NR_sysmips > { TARGET_NR_sysmips, "sysmips" , NULL, NULL, NULL }, > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index 05b4c41..a3e7d51 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -9339,14 +9339,51 @@ abi_long do_syscall(void *cpu_env, int num, abi_long > arg1, > ret = do_setsockopt(arg1, arg2, arg3, arg4, (socklen_t) arg5); > break; > #endif > - > +#if defined(TARGET_NR_syslog) > case TARGET_NR_syslog: > - if (!(p = lock_user_string(arg2))) > - goto efault; > - ret = get_errno(sys_syslog((int)arg1, p, (int)arg3)); > - unlock_user(p, arg2, 0); > - break; > + { > + int len = arg2; > > + switch (arg1) { > + case TARGET_SYSLOG_ACTION_CLOSE: /* Close log */ > + case TARGET_SYSLOG_ACTION_OPEN: /* Open log */ > + case TARGET_SYSLOG_ACTION_CLEAR: /* Clear ring buffer */ > + case TARGET_SYSLOG_ACTION_CONSOLE_OFF: /* Disable logging */ > + case TARGET_SYSLOG_ACTION_CONSOLE_ON: /* Enable logging */ > + case TARGET_SYSLOG_ACTION_CONSOLE_LEVEL: /* Set messages level */ > + case TARGET_SYSLOG_ACTION_SIZE_UNREAD: /* Number of chars */ > + case TARGET_SYSLOG_ACTION_SIZE_BUFFER: /* Size of the buffer */ > + { > + ret = get_errno(sys_syslog((int)arg1, NULL, (int)arg3)); > + } > + break; > + case TARGET_SYSLOG_ACTION_READ: /* Read from log */ > + case TARGET_SYSLOG_ACTION_READ_CLEAR: /* Read/clear msgs */ > + case TARGET_SYSLOG_ACTION_READ_ALL: /* Read last messages */ > + { > + if (len < 0) { > + ret = -TARGET_EINVAL; > + goto fail; > + } > + if (len == 0) { > + break; > + } > + p = lock_user(VERIFY_WRITE, arg2, arg3, 0); > + if (!p) { > + ret = -TARGET_EINVAL; > + goto fail; > + } > + ret = get_errno(sys_syslog((int)arg1, p, (int)arg3));
The error paths above are incorrect, compared to: http://lxr.free-electrons.com/source/kernel/printk/printk.c#L1465 I'll squash in the following change to match the kernel behaviour: diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 2072b1f..dfc483c 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -9377,16 +9377,17 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, case TARGET_SYSLOG_ACTION_READ_CLEAR: /* Read/clear msgs */ case TARGET_SYSLOG_ACTION_READ_ALL: /* Read last messages */ { + ret = -TARGET_EINVAL; if (len < 0) { - ret = -TARGET_EINVAL; goto fail; } + ret = 0; if (len == 0) { break; } p = lock_user(VERIFY_WRITE, arg2, arg3, 0); if (!p) { - ret = -TARGET_EINVAL; + ret = -TARGET_EFAULT; goto fail; } ret = get_errno(sys_syslog((int)arg1, p, (int)arg3)); > + unlock_user(p, arg2, arg3); > + } > + break; > + default: > + ret = -EINVAL; > + break; > + } > + } > + break; > +#endif > case TARGET_NR_setitimer: > { > struct itimerval value, ovalue, *pvalue; > diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h > index adb7153..61270ef 100644 > --- a/linux-user/syscall_defs.h > +++ b/linux-user/syscall_defs.h > @@ -2688,4 +2688,29 @@ struct target_user_cap_data { > uint32_t inheritable; > }; > > +/* from kernel's include/linux/syslog.h */ > + > +/* Close the log. Currently a NOP. */ > +#define TARGET_SYSLOG_ACTION_CLOSE 0 > +/* Open the log. Currently a NOP. */ > +#define TARGET_SYSLOG_ACTION_OPEN 1 > +/* Read from the log. */ > +#define TARGET_SYSLOG_ACTION_READ 2 > +/* Read all messages remaining in the ring buffer. */ > +#define TARGET_SYSLOG_ACTION_READ_ALL 3 > +/* Read and clear all messages remaining in the ring buffer */ > +#define TARGET_SYSLOG_ACTION_READ_CLEAR 4 > +/* Clear ring buffer. */ > +#define TARGET_SYSLOG_ACTION_CLEAR 5 > +/* Disable printk's to console */ > +#define TARGET_SYSLOG_ACTION_CONSOLE_OFF 6 > +/* Enable printk's to console */ > +#define TARGET_SYSLOG_ACTION_CONSOLE_ON 7 > +/* Set level of messages printed to console */ > +#define TARGET_SYSLOG_ACTION_CONSOLE_LEVEL 8 > +/* Return number of unread characters in the log buffer */ > +#define TARGET_SYSLOG_ACTION_SIZE_UNREAD 9 > +/* Return size of the log buffer */ > +#define TARGET_SYSLOG_ACTION_SIZE_BUFFER 10 > + > #endif > -- > 2.1.4 > >