Gentle ping.
--Owen
On Thu, Jul 1, 2021 at 3:12 PM Owen Anderson <oande...@google.com> wrote:
> The mapping from file-descriptors to translator functions is not guarded
> on realloc which may cause invalid function pointers to be read from a
> previously deallocated mapping.
>
> Signed-off-by: Owen Anderson <oande...@google.com>
> ---
> linux-user/fd-trans.c | 1 +
> linux-user/fd-trans.h | 55 +++++++++++++++++++++++++++++++++++++------
> linux-user/main.c | 3 +++
> 3 files changed, 52 insertions(+), 7 deletions(-)
>
> diff --git a/linux-user/fd-trans.c b/linux-user/fd-trans.c
> index 23adaca836..86b6f484d3 100644
> --- a/linux-user/fd-trans.c
> +++ b/linux-user/fd-trans.c
> @@ -267,6 +267,7 @@ enum {
> };
>
> TargetFdTrans **target_fd_trans;
> +QemuMutex target_fd_trans_lock;
> unsigned int target_fd_max;
>
> static void tswap_nlmsghdr(struct nlmsghdr *nlh)
> diff --git a/linux-user/fd-trans.h b/linux-user/fd-trans.h
> index a3fcdaabc7..1b9fa2041c 100644
> --- a/linux-user/fd-trans.h
> +++ b/linux-user/fd-trans.h
> @@ -16,6 +16,8 @@
> #ifndef FD_TRANS_H
> #define FD_TRANS_H
>
> +#include "qemu/lockable.h"
> +
> typedef abi_long (*TargetFdDataFunc)(void *, size_t);
> typedef abi_long (*TargetFdAddrFunc)(void *, abi_ulong, socklen_t);
> typedef struct TargetFdTrans {
> @@ -25,12 +27,23 @@ typedef struct TargetFdTrans {
> } TargetFdTrans;
>
> extern TargetFdTrans **target_fd_trans;
> +extern QemuMutex target_fd_trans_lock;
>
> extern unsigned int target_fd_max;
>
> +static inline void fd_trans_init(void)
> +{
> + qemu_mutex_init(&target_fd_trans_lock);
> +}
> +
> static inline TargetFdDataFunc fd_trans_target_to_host_data(int fd)
> {
> - if (fd >= 0 && fd < target_fd_max && target_fd_trans[fd]) {
> + if (fd < 0) {
> + return NULL;
> + }
> +
> + QEMU_LOCK_GUARD(&target_fd_trans_lock);
> + if (fd < target_fd_max && target_fd_trans[fd]) {
> return target_fd_trans[fd]->target_to_host_data;
> }
> return NULL;
> @@ -38,7 +51,12 @@ static inline TargetFdDataFunc
> fd_trans_target_to_host_data(int fd)
>
> static inline TargetFdDataFunc fd_trans_host_to_target_data(int fd)
> {
> - if (fd >= 0 && fd < target_fd_max && target_fd_trans[fd]) {
> + if (fd < 0) {
> + return NULL;
> + }
> +
> + QEMU_LOCK_GUARD(&target_fd_trans_lock);
> + if (fd < target_fd_max && target_fd_trans[fd]) {
> return target_fd_trans[fd]->host_to_target_data;
> }
> return NULL;
> @@ -46,13 +64,19 @@ static inline TargetFdDataFunc
> fd_trans_host_to_target_data(int fd)
>
> static inline TargetFdAddrFunc fd_trans_target_to_host_addr(int fd)
> {
> - if (fd >= 0 && fd < target_fd_max && target_fd_trans[fd]) {
> + if (fd < 0) {
> + return NULL;
> + }
> +
> + QEMU_LOCK_GUARD(&target_fd_trans_lock);
> + if (fd < target_fd_max && target_fd_trans[fd]) {
> return target_fd_trans[fd]->target_to_host_addr;
> }
> return NULL;
> }
>
> -static inline void fd_trans_register(int fd, TargetFdTrans *trans)
> +static inline void internal_fd_trans_register_unsafe(int fd,
> + TargetFdTrans *trans)
> {
> unsigned int oldmax;
>
> @@ -67,18 +91,35 @@ static inline void fd_trans_register(int fd,
> TargetFdTrans *trans)
> target_fd_trans[fd] = trans;
> }
>
> -static inline void fd_trans_unregister(int fd)
> +static inline void fd_trans_register(int fd, TargetFdTrans *trans)
> +{
> + QEMU_LOCK_GUARD(&target_fd_trans_lock);
> + internal_fd_trans_register_unsafe(fd, trans);
> +}
> +
> +static inline void internal_fd_trans_unregister_unsafe(int fd)
> {
> if (fd >= 0 && fd < target_fd_max) {
> target_fd_trans[fd] = NULL;
> }
> }
>
> +static inline void fd_trans_unregister(int fd)
> +{
> + if (fd < 0) {
> + return;
> + }
> +
> + QEMU_LOCK_GUARD(&target_fd_trans_lock);
> + internal_fd_trans_unregister_unsafe(fd);
> +}
> +
> static inline void fd_trans_dup(int oldfd, int newfd)
> {
> - fd_trans_unregister(newfd);
> + QEMU_LOCK_GUARD(&target_fd_trans_lock);
> + internal_fd_trans_unregister_unsafe(newfd);
> if (oldfd < target_fd_max && target_fd_trans[oldfd]) {
> - fd_trans_register(newfd, target_fd_trans[oldfd]);
> + internal_fd_trans_register_unsafe(newfd, target_fd_trans[oldfd]);
> }
> }
>
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 2fb3a366a6..37ed50d98e 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -48,6 +48,7 @@
> #include "target_elf.h"
> #include "cpu_loop-common.h"
> #include "crypto/init.h"
> +#include "fd-trans.h"
>
> #ifndef AT_FLAGS_PRESERVE_ARGV0
> #define AT_FLAGS_PRESERVE_ARGV0_BIT 0
> @@ -829,6 +830,8 @@ int main(int argc, char **argv, char **envp)
> cpu->opaque = ts;
> task_settid(ts);
>
> + fd_trans_init();
> +
> ret = loader_exec(execfd, exec_path, target_argv, target_environ,
> regs,
> info, &bprm);
> if (ret != 0) {
> --
> 2.32.0.93.g670b81a890-goog
>
>
