[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 06/22] linux-user: Fix syslog() syscall support
From: |
Riku Voipio |
Subject: |
Re: [Qemu-devel] [PULL 06/22] linux-user: Fix syslog() syscall support |
Date: |
Tue, 18 Oct 2016 08:01:19 +0000 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Mon, Oct 17, 2016 at 04:24:24PM +0300, address@hidden wrote:
> 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>
> Signed-off-by: Riku Voipio <address@hidden>
> ---
> 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
>
>
- [Qemu-devel] [PULL 00/22] linux-user changes, riku . voipio, 2016/10/17
- [Qemu-devel] [PULL 02/22] linux-user: Add support for ustat() syscall, riku . voipio, 2016/10/17
- [Qemu-devel] [PULL 01/22] linux-user: Add support for adjtimex() syscall, riku . voipio, 2016/10/17
- [Qemu-devel] [PULL 03/22] linux-user: Fix mq_open() syscall support, riku . voipio, 2016/10/17
- [Qemu-devel] [PULL 04/22] linux-user: Fix msgrcv() and msgsnd() syscalls support, riku . voipio, 2016/10/17
- [Qemu-devel] [PULL 08/22] linux-user: sparc64: Use correct target SHMLBA in shmat(), riku . voipio, 2016/10/17
- [Qemu-devel] [PULL 06/22] linux-user: Fix syslog() syscall support, riku . voipio, 2016/10/17
- Re: [Qemu-devel] [PULL 06/22] linux-user: Fix syslog() syscall support,
Riku Voipio <=
- [Qemu-devel] [PULL 11/22] linux-user: Don't use alloca() for epoll_wait's epoll event array, riku . voipio, 2016/10/17
- [Qemu-devel] [PULL 09/22] linux-user: add kcmp() syscall, riku . voipio, 2016/10/17
- [Qemu-devel] [PULL 05/22] linux-user: Fix socketcall() syscall support, riku . voipio, 2016/10/17
- [Qemu-devel] [PULL 07/22] linux-user: Remove a duplicate item from strace.list, riku . voipio, 2016/10/17
- [Qemu-devel] [PULL 10/22] linux-user: add RTA_PRIORITY in netlink, riku . voipio, 2016/10/17
- [Qemu-devel] [PULL 13/22] linux-user: Fix definition of target_sigevent for 32-bit guests, riku . voipio, 2016/10/17
- [Qemu-devel] [PULL 14/22] linux-user: Add support for clock_adjtime() syscall, riku . voipio, 2016/10/17
- [Qemu-devel] [PULL 15/22] linux-user: Add support for syncfs() syscall, riku . voipio, 2016/10/17
- [Qemu-devel] [PULL 12/22] linux-user: use libc wrapper instead of direct mremap syscall, riku . voipio, 2016/10/17
- [Qemu-devel] [PULL 19/22] linux-user: Fix fadvise64() syscall support for Mips32, riku . voipio, 2016/10/17