Re: [Qemu-devel] [linux-user] Added posix message queue syscalls except mq_notify

[Kirill A. Shutemov]

On Sat, Dec 13, 2008 at 01:39:27PM +0100, Lionel Landwerlin wrote:
> >From 57a528de47a737e59f391ff7df2f87367b40529e Mon Sep 17 00:00:00 2001
> From: Lionel Landwerlin <address@hidden>
> Date: Mon, 1 Dec 2008 02:42:24 +0100
> Subject: [PATCH] Added posix message queue syscalls except mq_notify
> 
> Signed-off-by: Lionel Landwerlin <address@hidden>
> 
> ---
>  linux-user/syscall.c |  151 
> ++++++++++++++++++++++++++++++++++++++++++++------
>  1 files changed, 117 insertions(+), 0 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 4065917..c4dd38a 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -28,6 +28,7 @@
>  #include <fcntl.h>
>  #include <time.h>
>  #include <limits.h>
> +#include <mqueue.h>
>  #include <sys/types.h>
>  #include <sys/ipc.h>
>  #include <sys/msg.h>
> @@ -629,6 +630,43 @@ static inline abi_long copy_to_user_timeval(abi_ulong 
> target_tv_addr,
>      return 0;
>  }
>  
> +static inline abi_long copy_from_user_mq_attr(struct mq_attr *attr,
> +                                              abi_ulong target_mq_attr_addr)
> +{
> +    struct mq_attr *target_mq_attr;

It's wrong. struct mq_attr has long int fields, so you should define
struct target_mq_attr using abi_long.

> +
> +    if (!lock_user_struct(VERIFY_READ, target_mq_attr,
> +                          target_mq_attr_addr, 1))
> +        return -TARGET_EFAULT;
> +
> +    __get_user(attr->mq_flags, &target_mq_attr->mq_flags);
> +    __get_user(attr->mq_maxmsg, &target_mq_attr->mq_maxmsg);
> +    __get_user(attr->mq_msgsize, &target_mq_attr->mq_msgsize);
> +    __get_user(attr->mq_curmsgs, &target_mq_attr->mq_curmsgs);
> +
> +    unlock_user_struct(target_mq_attr, target_mq_attr_addr, 0);
> +
> +    return 0;
> +}
> +
> +static inline abi_long copy_to_user_mq_attr(abi_ulong target_mq_attr_addr,
> +                                            const struct mq_attr *attr)
> +{
> +    struct mq_attr *target_mq_attr;
> +
> +    if (!lock_user_struct(VERIFY_WRITE, target_mq_attr,
> +                          target_mq_attr_addr, 0))
> +        return -TARGET_EFAULT;
> +
> +    __put_user(attr->mq_flags, &target_mq_attr->mq_flags);
> +    __put_user(attr->mq_maxmsg, &target_mq_attr->mq_maxmsg);
> +    __put_user(attr->mq_msgsize, &target_mq_attr->mq_msgsize);
> +    __put_user(attr->mq_curmsgs, &target_mq_attr->mq_curmsgs);
> +
> +    unlock_user_struct(target_mq_attr, target_mq_attr_addr, 1);
> +
> +    return 0;
> +}
>  
>  /* do_select() must return target values and target errnos. */
>  static abi_long do_select(int n,
> @@ -6033,6 +6071,85 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>          break;
>  #endif
>  
> +#ifdef TARGET_NR_mq_open
> +    case TARGET_NR_mq_open:
> +    {
> +        struct mq_attr posix_mq_attr;
> +
> +        p = lock_user_string(arg1 - 1);

Why - 1?

> +        if (arg4 != 0)
> +            copy_from_user_mq_attr (&posix_mq_attr, arg4);
> +        ret = get_errno(mq_open(p, arg2, arg3, &posix_mq_attr));
> +        unlock_user (p, arg1, 0);
> +        break;
> +    }
> +
> +    case TARGET_NR_mq_unlink:
> +        p = lock_user_string(arg1 - 1);

?

> +        ret = get_errno(mq_unlink(p));
> +        unlock_user (p, arg1, 0);
> +        break;
> +
> +    case TARGET_NR_mq_timedsend:
> +    {
> +        struct timespec ts;
> +
> +        if (arg5 != 0) {
> +            p = lock_user (VERIFY_READ, arg2, arg3, 1);
> +            target_to_host_timespec(&ts, arg5);
> +            ret = get_errno(mq_timedsend(arg1, p, arg3, arg4, &ts));
> +            host_to_target_timespec(arg5, &ts);
> +            unlock_user (p, arg2, arg3);
> +        } else {
> +            p = lock_user (VERIFY_READ, arg2, arg3, 1);
> +            ret = get_errno(mq_send(arg1, p, arg3, arg4));
> +            unlock_user (p, arg2, arg3);
> +        }

We can lock and unlock outside of if startament, I think.

> +        break;
> +    }
> +
> +    case TARGET_NR_mq_timedreceive:
> +    {
> +        struct timespec ts;
> +        unsigned int prio;
> +
> +        if (arg5 != 0) {
> +            p = lock_user (VERIFY_READ, arg2, arg3, 1);
> +            target_to_host_timespec(&ts, arg5);
> +            ret = get_errno(mq_timedreceive(arg1, p, arg3, &prio, &ts));
> +            host_to_target_timespec(arg5, &ts);
> +            unlock_user (p, arg2, arg3);
> +        } else {
> +            p = lock_user (VERIFY_READ, arg2, arg3, 1);
> +            ret = get_errno(mq_receive(arg1, p, arg3, &prio));
> +            unlock_user (p, arg2, arg3);
> +        }

The same about locking.

> +        if (arg4 != 0)
> +            put_user_u32(prio, arg4);
> +        break;
> +    }
> +
> +    /* Not implemented for now... */
> +/*     case TARGET_NR_mq_notify: */
> +/*         break; */

Is there any problem with this syscall?

> +
> +    case TARGET_NR_mq_getsetattr:
> +    {
> +        struct mq_attr posix_mq_attr_in, posix_mq_attr_out;
> +
> +        if (arg3 != 0) {
> +            ret = mq_getattr(arg1, &posix_mq_attr_out);
> +            copy_to_user_mq_attr(arg3, &posix_mq_attr_out);
> +        }
> +        if (arg2 != 0) {
> +            copy_from_user_mq_attr(&posix_mq_attr_in, arg2);
> +            ret |= mq_setattr(arg1, &posix_mq_attr_in, &posix_mq_attr_out);
> +        }
> +
> +        break;
> +    }
> +#endif
> +
>      default:
>      unimplemented:
>          gemu_log("qemu: Unsupported syscall: %d\n", num);
> -- 
> 1.5.6.5
-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.org/

signature.asc
Description: Digital signature