[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] linux-user: Fix length calculations in host
From: |
Laurent Vivier |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] linux-user: Fix length calculations in host_to_target_cmsg() |
Date: |
Fri, 19 Jan 2018 17:32:50 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 |
Le 15/12/2017 à 14:52, Peter Maydell a écrit :
> The handling of length calculations in host_to_target_cmsg()
> was rather confused:
> * when checking for whether the target cmsg header fit in
> the remaining buffer, we were using the host struct size,
> not the target size
> * we were setting tgt_len to "target payload + header length"
> but then using it as if it were the target payload length alone
> * in various message type cases we weren't handling the possibility
> that host or target buffers were truncated
>
> Fix these problems. The second one in particular is liable
> to result in us overrunning the guest provided buffer,
> since we will try to convert more data than is actually
> present.
>
> Fixes: https://bugs.launchpad.net/qemu/+bug/1701808
> Reported-by: Bruno Haible <address@hidden>
> Signed-off-by: Peter Maydell <address@hidden>
> ---
> linux-user/syscall.c | 29 ++++++++++++++++++++++-------
> 1 file changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 11c9116..a1b9772 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1782,7 +1782,7 @@ static inline abi_long host_to_target_cmsg(struct
> target_msghdr *target_msgh,
> * to the guest via the CTRUNC bit), unlike truncation
> * in target_to_host_cmsg, which is a QEMU bug.
> */
> - if (msg_controllen < sizeof(struct cmsghdr)) {
> + if (msg_controllen < sizeof(struct target_cmsghdr)) {
> target_msgh->msg_flags |= tswap32(MSG_CTRUNC);
> break;
> }
> @@ -1794,8 +1794,6 @@ static inline abi_long host_to_target_cmsg(struct
> target_msghdr *target_msgh,
> }
> target_cmsg->cmsg_type = tswap32(cmsg->cmsg_type);
>
> - tgt_len = TARGET_CMSG_LEN(len);
> -
> /* Payload types which need a different size of payload on
> * the target must adjust tgt_len here.
> */
> @@ -1809,12 +1807,13 @@ static inline abi_long host_to_target_cmsg(struct
> target_msghdr *target_msgh,
> break;
> }
> default:
> + tgt_len = len;
> break;
> }
>
> - if (msg_controllen < tgt_len) {
> + if (msg_controllen < TARGET_CMSG_LEN(tgt_len)) {
> target_msgh->msg_flags |= tswap32(MSG_CTRUNC);
> - tgt_len = msg_controllen;
> + tgt_len = msg_controllen - sizeof(struct target_cmsghdr);
> }
>
> /* We must now copy-and-convert len bytes of payload
> @@ -1875,6 +1874,10 @@ static inline abi_long host_to_target_cmsg(struct
> target_msghdr *target_msgh,
> uint32_t *v = (uint32_t *)data;
> uint32_t *t_int = (uint32_t *)target_data;
>
> + if (len != sizeof(uint32_t) ||
> + tgt_len != sizeof(uint32_t)) {
> + goto unimplemented;
> + }
> __put_user(*v, t_int);
> break;
> }
> @@ -1888,6 +1891,10 @@ static inline abi_long host_to_target_cmsg(struct
> target_msghdr *target_msgh,
> struct errhdr_t *target_errh =
> (struct errhdr_t *)target_data;
>
> + if (len != sizeof(struct errhdr_t) ||
> + tgt_len != sizeof(struct errhdr_t)) {
> + goto unimplemented;
> + }
> __put_user(errh->ee.ee_errno, &target_errh->ee.ee_errno);
> __put_user(errh->ee.ee_origin, &target_errh->ee.ee_origin);
> __put_user(errh->ee.ee_type, &target_errh->ee.ee_type);
> @@ -1911,6 +1918,10 @@ static inline abi_long host_to_target_cmsg(struct
> target_msghdr *target_msgh,
> uint32_t *v = (uint32_t *)data;
> uint32_t *t_int = (uint32_t *)target_data;
>
> + if (len != sizeof(uint32_t) ||
> + tgt_len != sizeof(uint32_t)) {
> + goto unimplemented;
> + }
> __put_user(*v, t_int);
> break;
> }
> @@ -1924,6 +1935,10 @@ static inline abi_long host_to_target_cmsg(struct
> target_msghdr *target_msgh,
> struct errhdr6_t *target_errh =
> (struct errhdr6_t *)target_data;
>
> + if (len != sizeof(struct errhdr6_t) ||
> + tgt_len != sizeof(struct errhdr6_t)) {
> + goto unimplemented;
> + }
> __put_user(errh->ee.ee_errno, &target_errh->ee.ee_errno);
> __put_user(errh->ee.ee_origin, &target_errh->ee.ee_origin);
> __put_user(errh->ee.ee_type, &target_errh->ee.ee_type);
> @@ -1950,8 +1965,8 @@ static inline abi_long host_to_target_cmsg(struct
> target_msghdr *target_msgh,
> }
> }
>
> - target_cmsg->cmsg_len = tswapal(tgt_len);
> - tgt_space = TARGET_CMSG_SPACE(len);
> + target_cmsg->cmsg_len = tswapal(TARGET_CMSG_LEN(tgt_len));
> + tgt_space = TARGET_CMSG_SPACE(tgt_len);
> if (msg_controllen < tgt_space) {
> tgt_space = msg_controllen;
> }
>
Applied to my linux-user branch.
Thanks,
Laurent
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH 1/2] linux-user: Fix length calculations in host_to_target_cmsg(),
Laurent Vivier <=