[PATCH] linux-user: always translate cmsg when recvmsg

[PATCH] linux-user: always translate cmsg when recvmsg

[Icenowy Zheng]

It's possible that a message contains both normal payload and ancillary
data in the same message, and even if no ancillary data is available
this information should be passed to the target, otherwise the target
cmsghdr will be left uninitialized and the target is going to access
uninitialized memory if it expects cmsg.

Always call the function that translate cmsg when recvmsg, because that
function should be empty-cmsg-safe (it creates an empty cmsg in the
target).

Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
---
 linux-user/syscall.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 8402c1399d..029a4e8b42 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3346,7 +3346,8 @@ static abi_long do_sendrecvmsg_locked(int fd, struct target_msghdr *msgp,
             if (fd_trans_host_to_target_data(fd)) {
                 ret = fd_trans_host_to_target_data(fd)(msg.msg_iov->iov_base,
                                                MIN(msg.msg_iov->iov_len, len));
-            } else {
+            }
+            if (!is_error(ret)) {
                 ret = host_to_target_cmsg(msgp, &msg);
             }
             if (!is_error(ret)) {
-- 
2.37.1

Re: [PATCH] linux-user: always translate cmsg when recvmsg

[Icenowy Zheng]

在 2022-10-28星期五的 16:12 +0800，Icenowy Zheng写道：
> It's possible that a message contains both normal payload and
> ancillary
> data in the same message, and even if no ancillary data is available
> this information should be passed to the target, otherwise the target
> cmsghdr will be left uninitialized and the target is going to access
> uninitialized memory if it expects cmsg.

Here attaches an full example that utilizes RTNL and NETLINK_PKTINFO
cmsg:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <unistd.h>
#include <linux/netlink.h>
#include <linux/rtnetlink.h>

struct rtnlreq {
        struct nlmsghdr nmh;
        struct rtgenmsg msg;
};

int main()
{
        int fd, len, sockopt;
        struct sockaddr_nl local, remote;
        struct rtnlreq req;
        struct nlmsghdr *resp;
        struct cmsghdr *cmh;
        struct iovec iov;
        struct msghdr mh;
        char respbuf[8192],cmsgbuf[32];

        fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);

        memset(&local, 0, sizeof(local));
        local.nl_family = AF_NETLINK;
        local.nl_pid = getpid();
        local.nl_groups = 0;

        if (bind(fd, (struct sockaddr *)&local, sizeof(local)) < 0)
                abort();

        sockopt = 1;
        setsockopt(fd, SOL_NETLINK, NETLINK_PKTINFO, &sockopt,
	sizeof(sockopt));

        memset(&remote, 0, sizeof(remote));
        remote.nl_family = AF_NETLINK;
        remote.nl_groups = 0;

        memset(&req, 0, sizeof(req));
        req.nmh.nlmsg_len = NLMSG_LENGTH(sizeof(req.msg));
        req.nmh.nlmsg_type = RTM_GETLINK;
        req.nmh.nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP; 
        req.nmh.nlmsg_seq = 1;
        req.nmh.nlmsg_pid = getpid();
        req.msg.rtgen_family = AF_INET;

        iov.iov_base = &req;
        iov.iov_len = req.nmh.nlmsg_len;

        memset(&mh, 0, sizeof(mh));
        mh.msg_iov = &iov;
        mh.msg_iovlen = 1;
        mh.msg_name = &remote;
        mh.msg_namelen = sizeof(remote);

        sendmsg(fd, &mh, 0);

        iov.iov_base = respbuf;
        iov.iov_len = 8192;

        memset(&mh, 0, sizeof(mh));
        mh.msg_iov = &iov;
        mh.msg_iovlen = 1;
        mh.msg_name = &remote;
        mh.msg_namelen = sizeof(remote);
        mh.msg_control = cmsgbuf;
        mh.msg_controllen = 32;

        len = recvmsg(fd, &mh, 0);
        if (!len)
                abort();

        resp = (struct nlmsghdr *)respbuf;
        cmh = CMSG_FIRSTHDR(&mh);
        if (NLMSG_OK(resp, len)) {
                printf("resp type %d len %zd\n",
                       resp->nlmsg_type, resp->nlmsg_len);
        }
        if (cmh) {
                printf("cmsg level %d type %d len %zd\n",
                       cmh->cmsg_level, cmh->cmsg_type, cmh->cmsg_len);
        }
}

On my machine, when it runs natively or runs under patched qemu, the
printed info is:
```
resp type 16 len 1392
cmsg level 270 type 3 len 20
```

However, with unpatched qemu it is:
```
resp type 16 len 1392
cmsg level 0 type 0 len 0
```

> 
> Always call the function that translate cmsg when recvmsg, because
> that
> function should be empty-cmsg-safe (it creates an empty cmsg in the
> target).
> 
> Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> ---
>  linux-user/syscall.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 8402c1399d..029a4e8b42 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -3346,7 +3346,8 @@ static abi_long do_sendrecvmsg_locked(int fd,
> struct target_msghdr *msgp,
>              if (fd_trans_host_to_target_data(fd)) {
>                  ret = fd_trans_host_to_target_data(fd)(msg.msg_iov-
> >iov_base,
>                                                 MIN(msg.msg_iov-
> >iov_len, len));
> -            } else {
> +            }
> +            if (!is_error(ret)) {
>                  ret = host_to_target_cmsg(msgp, &msg);
>              }
>              if (!is_error(ret)) {

Re: [PATCH] linux-user: always translate cmsg when recvmsg

[Laurent Vivier]

Le 28/10/2022 à 10:12, Icenowy Zheng a écrit :
> It's possible that a message contains both normal payload and ancillary
> data in the same message, and even if no ancillary data is available
> this information should be passed to the target, otherwise the target
> cmsghdr will be left uninitialized and the target is going to access
> uninitialized memory if it expects cmsg.
> 
> Always call the function that translate cmsg when recvmsg, because that
> function should be empty-cmsg-safe (it creates an empty cmsg in the
> target).
> 
> Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> ---
>   linux-user/syscall.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH] linux-user: always translate cmsg when recvmsg

[Laurent Vivier]

Le 28/10/2022 à 10:12, Icenowy Zheng a écrit :
> It's possible that a message contains both normal payload and ancillary
> data in the same message, and even if no ancillary data is available
> this information should be passed to the target, otherwise the target
> cmsghdr will be left uninitialized and the target is going to access
> uninitialized memory if it expects cmsg.
> 
> Always call the function that translate cmsg when recvmsg, because that
> function should be empty-cmsg-safe (it creates an empty cmsg in the
> target).
> 
> Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> ---
>   linux-user/syscall.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 8402c1399d..029a4e8b42 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -3346,7 +3346,8 @@ static abi_long do_sendrecvmsg_locked(int fd, struct target_msghdr *msgp,
>               if (fd_trans_host_to_target_data(fd)) {
>                   ret = fd_trans_host_to_target_data(fd)(msg.msg_iov->iov_base,
>                                                  MIN(msg.msg_iov->iov_len, len));
> -            } else {
> +            }
> +            if (!is_error(ret)) {
>                   ret = host_to_target_cmsg(msgp, &msg);
>               }
>               if (!is_error(ret)) {

Applied to my linux-user-for-7.2 branch.

Thanks,
Laurent