Re: [Qemu-devel] [PATCH v2] linux-user: Allocate thunk size dynamically

[Riku Voipio]

On 7 May 2015 at 00:47, Alexander Graf <address@hidden> wrote:
> We store all struct types in an array of static size without ever
> checking whether we overrun it. Of course some day someone (like me
> in another, ancient ALSA enabling patch set) will run into the limit
> without realizing it.
>
> So let's make the allocation dynamic. We already know the number of
> structs that we want to allocate, so we only need to pass the variable
> into the respective piece of code.
>
> Also, to ensure we don't accidently overwrite random memory, add some
> asserts to sanity check whether a thunk is actually part of our array.
>
> Signed-off-by: Alexander Graf <address@hidden>

Applied,

thanks

> ---
>
> v1 -> v2:
>
>   - alloc with new0 to copy the bss semantics we had before and
>     implicitly initialize se->convert[0,1] to 0
> ---
>  include/exec/user/thunk.h |  4 +++-
>  linux-user/syscall.c      |  3 +++
>  thunk.c                   | 16 ++++++++++++----
>  3 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/include/exec/user/thunk.h b/include/exec/user/thunk.h
> index 87025c3..3b67462 100644
> --- a/include/exec/user/thunk.h
> +++ b/include/exec/user/thunk.h
> @@ -74,7 +74,7 @@ const argtype *thunk_convert(void *dst, const void *src,
>                               const argtype *type_ptr, int to_host);
>  #ifndef NO_THUNK_TYPE_SIZE
>
> -extern StructEntry struct_entries[];
> +extern StructEntry *struct_entries;
>
>  int thunk_type_size_array(const argtype *type_ptr, int is_host);
>  int thunk_type_align_array(const argtype *type_ptr, int is_host);
> @@ -186,4 +186,6 @@ unsigned int target_to_host_bitmask(unsigned int x86_mask,
>  unsigned int host_to_target_bitmask(unsigned int alpha_mask,
>                                      const bitmask_transtbl * trans_tbl);
>
> +void thunk_init(unsigned int max_structs);
> +
>  #endif
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 1622ad6..f56f3e0 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -3277,6 +3277,7 @@ static abi_long do_ipc(unsigned int call, abi_long 
> first,
>  #define STRUCT_SPECIAL(name) STRUCT_ ## name,
>  enum {
>  #include "syscall_types.h"
> +STRUCT_MAX
>  };
>  #undef STRUCT
>  #undef STRUCT_SPECIAL
> @@ -4879,6 +4880,8 @@ void syscall_init(void)
>      int size;
>      int i;
>
> +    thunk_init(STRUCT_MAX);
> +
>  #define STRUCT(name, ...) thunk_register_struct(STRUCT_ ## name, #name, 
> struct_ ## name ## _def);
>  #define STRUCT_SPECIAL(name) thunk_register_struct_direct(STRUCT_ ## name, 
> #name, &struct_ ## name ## _def);
>  #include "syscall_types.h"
> diff --git a/thunk.c b/thunk.c
> index 3cca047..f501fd7 100644
> --- a/thunk.c
> +++ b/thunk.c
> @@ -25,10 +25,8 @@
>
>  //#define DEBUG
>
> -#define MAX_STRUCTS 128
> -
> -/* XXX: make it dynamic */
> -StructEntry struct_entries[MAX_STRUCTS];
> +static unsigned int max_struct_entries;
> +StructEntry *struct_entries;
>
>  static const argtype *thunk_type_next_ptr(const argtype *type_ptr);
>
> @@ -70,6 +68,7 @@ void thunk_register_struct(int id, const char *name, const 
> argtype *types)
>      StructEntry *se;
>      int nb_fields, offset, max_align, align, size, i, j;
>
> +    assert(id < max_struct_entries);
>      se = struct_entries + id;
>
>      /* first we count the number of fields */
> @@ -117,6 +116,8 @@ void thunk_register_struct_direct(int id, const char 
> *name,
>                                    const StructEntry *se1)
>  {
>      StructEntry *se;
> +
> +    assert(id < max_struct_entries);
>      se = struct_entries + id;
>      *se = *se1;
>      se->name = name;
> @@ -244,6 +245,7 @@ const argtype *thunk_convert(void *dst, const void *src,
>              const argtype *field_types;
>              const int *dst_offsets, *src_offsets;
>
> +            assert(*type_ptr < max_struct_entries);
>              se = struct_entries + *type_ptr++;
>              if (se->convert[0] != NULL) {
>                  /* specific conversion is needed */
> @@ -314,3 +316,9 @@ int thunk_type_align_array(const argtype *type_ptr, int 
> is_host)
>      return thunk_type_align(type_ptr, is_host);
>  }
>  #endif /* ndef NO_THUNK_TYPE_SIZE */
> +
> +void thunk_init(unsigned int max_structs)
> +{
> +    max_struct_entries = max_structs;
> +    struct_entries = g_new0(StructEntry, max_structs);
> +}
> --
> 1.7.12.4
>