Re: [PATCH v3 3/3] linux-user/syscall.c: do_ppoll: eliminate large alloca

[Daniel P . Berrangé]

On Thu, Sep 14, 2023 at 10:43:37AM +0300, Michael Tokarev wrote:
> do_ppoll() in linux-user/syscall.c uses alloca() to allocate
> an array of struct pullfds on the stack.  The only upper
> boundary for number of entries for this array is so that
> whole thing fits in INT_MAX.  This is definitely too much
> for stack allocation.
> 
> Use heap allocation when large number of entries is requested
> (currently 32, arbitrary), and continue to use alloca() for

Typo ? The code uses 64 rather than 32.

> smaller allocations, to optimize small operations for small
> sizes.  The code for this optimization is small, I see no
> reason for dropping it.
> 
> This eliminates last large user-controlled on-stack allocation
> from syscall.c.
> 
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
>  linux-user/syscall.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index eabdf50abc..1dbe28eba4 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1489,7 +1489,7 @@ static abi_long do_ppoll(abi_long arg1, abi_long arg2, 
> abi_long arg3,
>  {
>      struct target_pollfd *target_pfd = NULL;
>      unsigned int nfds = arg2;
> -    struct pollfd *pfd = NULL;
> +    struct pollfd *pfd = NULL, *heap_pfd = NULL;

g_autofree struct pollfd *heap_pdf = NULL;

>      unsigned int i;
>      abi_long ret;
>  
> @@ -1503,7 +1503,17 @@ static abi_long do_ppoll(abi_long arg1, abi_long arg2, 
> abi_long arg3,
>              return -TARGET_EFAULT;
>          }
>  
> -        pfd = alloca(sizeof(struct pollfd) * nfds);
> +        /* arbitrary "small" number to limit stack usage */
> +        if (nfds <= 64) {
> +            pfd = alloca(sizeof(struct pollfd) * nfds);
> +        } else {
> +            heap_pfd = g_try_new(struct pollfd, nfds);
> +            if (!heap_pfd) {
> +                ret = -TARGET_ENOMEM;
> +                goto out;
> +            }
> +            pfd = heap_pfd;
> +        }
>          for (i = 0; i < nfds; i++) {
>              pfd[i].fd = tswap32(target_pfd[i].fd);
>              pfd[i].events = tswap16(target_pfd[i].events);
> @@ -1567,6 +1577,7 @@ static abi_long do_ppoll(abi_long arg1, abi_long arg2, 
> abi_long arg3,
>      }
>  
>  out:
> +    g_free(heap_pfd);

This can be dropped with g_autofree usage

>      unlock_user(target_pfd, arg1, sizeof(struct target_pollfd) * nfds);
>      return ret;
>  }
> -- 
> 2.39.2
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|