diff mbox

[PULL,06/22] linux-user: Fix syslog() syscall support

Message ID 20161018080119.GA19984@kos.to
State New
Headers show

Commit Message

Riku Voipio Oct. 18, 2016, 8:01 a.m. UTC 
On Mon, Oct 17, 2016 at 04:24:24PM +0300, riku.voipio@linaro.org wrote:
> 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>

> Signed-off-by: Riku Voipio <riku.voipio@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:




> +                    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

> 

>
diff mbox

Patch

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));