Re: [PATCH v2 4/9] bsd-user/syscall: Replace alloca() by g_new()

[Eric Blake <eblake@redhat.com>]

On 5/6/21 9:16 AM, Warner Losh wrote:
> On Thu, May 6, 2021, 7:38 AM Philippe Mathieu-Daudé <philmd@redhat.com>
> wrote:
> 
>> The ALLOCA(3) man-page mentions its "use is discouraged".
>>
>> Replace it by a g_new() call.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  bsd-user/syscall.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/bsd-user/syscall.c b/bsd-user/syscall.c
>> index 4abff796c76..dbee0385ceb 100644
>> --- a/bsd-user/syscall.c
>> +++ b/bsd-user/syscall.c
>> @@ -355,9 +355,8 @@ abi_long do_freebsd_syscall(void *cpu_env, int num,
>> abi_long arg1,
>>      case TARGET_FREEBSD_NR_writev:
>>          {
>>              int count = arg3;
>> -            struct iovec *vec;
>> +            g_autofree struct iovec *vec = g_new(struct iovec, count);
>>
> 
> Where is this freed? Also, alloca just moves a stack pointer, where malloc
> has complex interactions. Are you sure that's a safe change here?

It's freed any time the g_autofree variable goes out of scope (that's
what the g_autofree macro is for).  Yes, the change is safe, although
you are right that switching to malloc is going to be a bit more
heavyweight than what alloca used.  What's more, it adds safety: if
count was under user control, a user could pass a value that could cause
alloca to allocate more than 4k and accidentally mess up stack guard
pages, while malloc() uses the heap and therefore cannot cause stack bugs.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org