diff mbox

[v5,7/8] linux-user: Fix syslog() syscall support

Message ID 20160914202008.14119-8-aleksandar.markovic@rt-rk.com (mailing list archive)
State New, archived
Headers show

Commit Message

Aleksandar Markovic Sept. 14, 2016, 8:20 p.m. UTC 
From: Aleksandar Markovic <aleksandar.markovic@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.markovic@imgtec.com>
---
 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(-)

Comments

Laurent Vivier Sept. 18, 2016, 12:38 a.m. UTC | #1 
Le 14/09/2016 à 22:20, Aleksandar Markovic a écrit :
> From: Aleksandar Markovic <aleksandar.markovic@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.markovic@imgtec.com>
> ---
>  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
>
diff mbox

Patch

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)
+{
+    switch (arg) {
+        case TARGET_SYSLOG_ACTION_CLOSE: {
+            gemu_log("%s%s", "SYSLOG_ACTION_CLOSE", get_comma(last));
+            break;
+        }
+        case TARGET_SYSLOG_ACTION_OPEN: {
+            gemu_log("%s%s", "SYSLOG_ACTION_OPEN", get_comma(last));
+            break;
+        }
+        case TARGET_SYSLOG_ACTION_READ: {
+            gemu_log("%s%s", "SYSLOG_ACTION_READ", get_comma(last));
+            break;
+        }
+        case TARGET_SYSLOG_ACTION_READ_ALL: {
+            gemu_log("%s%s", "SYSLOG_ACTION_READ_ALL", get_comma(last));
+            break;
+        }
+        case TARGET_SYSLOG_ACTION_READ_CLEAR: {
+            gemu_log("%s%s", "SYSLOG_ACTION_READ_CLEAR", get_comma(last));
+            break;
+        }
+        case TARGET_SYSLOG_ACTION_CLEAR: {
+            gemu_log("%s%s", "SYSLOG_ACTION_CLEAR", get_comma(last));
+            break;
+        }
+        case TARGET_SYSLOG_ACTION_CONSOLE_OFF: {
+            gemu_log("%s%s", "SYSLOG_ACTION_CONSOLE_OFF", get_comma(last));
+            break;
+        }
+        case TARGET_SYSLOG_ACTION_CONSOLE_ON: {
+            gemu_log("%s%s", "SYSLOG_ACTION_CONSOLE_ON", get_comma(last));
+            break;
+        }
+        case TARGET_SYSLOG_ACTION_CONSOLE_LEVEL: {
+            gemu_log("%s%s", "SYSLOG_ACTION_CONSOLE_LEVEL", get_comma(last));
+            break;
+        }
+        case TARGET_SYSLOG_ACTION_SIZE_UNREAD: {
+            gemu_log("%s%s", "SYSLOG_ACTION_SIZE_UNREAD", get_comma(last));
+            break;
+        }
+        case TARGET_SYSLOG_ACTION_SIZE_BUFFER: {
+            gemu_log("%s%s", "SYSLOG_ACTION_SIZE_BUFFER", get_comma(last));
+            break;
+        }
+        default: {
+            print_raw_param("%ld", arg, 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);
+                if (!p) {
+                    ret = -TARGET_EINVAL;
+                    goto fail;
+                }
+                ret = get_errno(sys_syslog((int)arg1, p, (int)arg3));
+                unlock_user(p, arg2, 0);
+            } 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