[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 7/8] linux-user: Fix syslog() syscall support
From: |
Laurent Vivier |
Subject: |
Re: [Qemu-devel] [PATCH v5 7/8] linux-user: Fix syslog() syscall support |
Date: |
Sun, 18 Sep 2016 02:38:41 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 |
Le 14/09/2016 à 22:20, Aleksandar Markovic a écrit :
> From: Aleksandar Markovic <address@hidden>
>
> 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 <address@hidden>
> ---
> linux-user/strace.c | 68
> +++++++++++++++++++++++++++++++++++++++++++++++
> linux-user/strace.list | 2 +-
> linux-user/syscall.c | 23 +++++++++++-----
> linux-user/syscall_defs.h | 25 +++++++++++++++++
> 4 files changed, 111 insertions(+), 7 deletions(-)
>
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 61911e7..6177f2c 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -1709,6 +1709,74 @@ print_rt_sigprocmask(const struct syscallname *name,
> }
> #endif
>
> +#ifdef TARGET_NR_syslog
> +static void
> +print_syslog_action(abi_ulong arg, int last)
> +{
The double "%s" and duplicate gemu_log() are not really nice.
you can do:
const char *type;
> + switch (arg) {
> + case TARGET_SYSLOG_ACTION_CLOSE: {
> + gemu_log("%s%s", "SYSLOG_ACTION_CLOSE", get_comma(last));
type = "SYSLOG_ACTION_CLOSE";
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 0bf1bea..2f99ac2 100644
> --- a/linux-user/strace.list
> +++ b/linux-user/strace.list
> @@ -1380,7 +1380,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 4ffcce5..37ce908 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9219,14 +9219,25 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
> arg1,
> ret = do_setsockopt(arg1, arg2, arg3, arg4, (socklen_t) arg5);
> break;
> #endif
> -
> +#ifdef 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);
> + {
> + if (((int)arg1 == TARGET_SYSLOG_ACTION_READ) ||
> + ((int)arg1 == TARGET_SYSLOG_ACTION_READ_ALL) ||
> + ((int)arg1 == TARGET_SYSLOG_ACTION_READ_CLEAR)) {
> + p = lock_user_string(arg2);
arg2 is empty. length is given by arg3.
Use lock_user(VERIFY_WRITE, arg2, arg3, 0) for that.
> + if (!p) {
> + ret = -TARGET_EINVAL;
> + goto fail;
> + }
> + ret = get_errno(sys_syslog((int)arg1, p, (int)arg3));
> + unlock_user(p, arg2, 0);
unlock_user(p, arg2, arg3);
> + } else {
> + ret = get_errno(sys_syslog((int)arg1, NULL, (int)arg3));
> + }
> + }
> 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 afd9191..50b1b60 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -2661,4 +2661,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
>
- Re: [Qemu-devel] [PATCH v5 2/8] linux-user: Add support for clock_adjtime() syscall, (continued)
- [Qemu-devel] [PATCH v5 3/8] linux-user: Add support for sysfs() syscall, Aleksandar Markovic, 2016/09/14
- [Qemu-devel] [PATCH v5 4/8] linux-user: Add support for ustat() syscall, Aleksandar Markovic, 2016/09/14
- [Qemu-devel] [PATCH v5 5/8] linux-user: Fix msgrcv() and msgsnd() syscalls support, Aleksandar Markovic, 2016/09/14
- [Qemu-devel] [PATCH v5 6/8] linux-user: Fix socketcall() syscall support, Aleksandar Markovic, 2016/09/14
- [Qemu-devel] [PATCH v5 7/8] linux-user: Fix syslog() syscall support, Aleksandar Markovic, 2016/09/14
- Re: [Qemu-devel] [PATCH v5 7/8] linux-user: Fix syslog() syscall support,
Laurent Vivier <=
- [Qemu-devel] [PATCH v5 8/8] linux-user: Remove a duplicate item from strace.list, Aleksandar Markovic, 2016/09/14