Le 14/09/2016 à 22:20, Aleksandar Markovic a écrit :
> 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>
> ---
>  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
> 

Reply via email to