[igt-dev] [PATCH i-g-t] lib: Assert potential malloc failures in intel_batchbuffer

[Zbigniew Kempczyński]

On Sat, Nov 21, 2020 at 02:50:32PM +0000, Chris Wilson wrote:
> Hunting:
> 
> 	Received signal SIGSEGV.
> 	Stack trace:
> 	 #0 [fatal_sig_handler+0xd6]
> 	 #1 [killpg+0x40]
> 	 #2 [intel_bb_add_object+0x105]
> 	 #3 [__real_main666+0xe83]
> 	 #4 [main+0x27]
> 	 #5 [__libc_start_main+0xe7]
> 	 #6 [_start+0x2a]
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  lib/intel_batchbuffer.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
> index 8dd8a5027..48cca852f 100644
> --- a/lib/intel_batchbuffer.c
> +++ b/lib/intel_batchbuffer.c
> @@ -1190,19 +1190,18 @@ static bool intel_bb_debug_tree = false;
>   */
>  static void __reallocate_objects(struct intel_bb *ibb)
>  {
> -	uint32_t num;
> +	const uint32_t inc = 4096 / sizeof(*ibb->objects);
>  
>  	if (ibb->num_objects == ibb->allocated_objects) {
> -		num = 4096 / sizeof(*ibb->objects);
>  		ibb->objects = realloc(ibb->objects,
>  				       sizeof(*ibb->objects) *
> -				       (num + ibb->allocated_objects));
> +				       (inc + ibb->allocated_objects));
>  
>  		igt_assert(ibb->objects);
> -		ibb->allocated_objects += num;
> +		ibb->allocated_objects += inc;
>  
>  		memset(&ibb->objects[ibb->num_objects],	0,
> -		       num * sizeof(*ibb->objects));
> +		       inc * sizeof(*ibb->objects));
>  	}
>  }
>  
> @@ -1592,6 +1591,8 @@ __add_to_cache(struct intel_bb *ibb, uint32_t handle)
>  	struct drm_i915_gem_exec_object2 **found, *object;
>  
>  	object = malloc(sizeof(*object));
> +	igt_assert(object);
> +
>  	object->handle = handle;
>  	found = tsearch((void *) object, &ibb->root, __compare_objects);
>  
> @@ -1615,16 +1616,18 @@ static int __compare_handles(const void *p1, const void *p2)
>  static void __add_to_objects(struct intel_bb *ibb,
>  			     struct drm_i915_gem_exec_object2 *object)
>  {
> -	uint32_t i, **found, *handle;
> +	uint32_t **found, *handle;
>  
>  	handle = malloc(sizeof(*handle));
> +	igt_assert(handle);
> +
>  	*handle = object->handle;
>  	found = tsearch((void *) handle, &ibb->current, __compare_handles);
>  
>  	if (*found == handle) {
>  		__reallocate_objects(ibb);
> -		i = ibb->num_objects++;
> -		ibb->objects[i] = object;
> +		igt_assert(ibb->num_objects < ibb->allocated_objects);
> +		ibb->objects[ibb->num_objects++] = object;
>  	} else {
>  		free(handle);
>  	}
> @@ -2118,7 +2121,7 @@ static int __intel_bb_exec(struct intel_bb *ibb, uint32_t end_offset,
>  
>  	memset(&execbuf, 0, sizeof(execbuf));
>  	objects = create_objects_array(ibb);
> -	execbuf.buffers_ptr = (uintptr_t) objects;
> +	execbuf.buffers_ptr = to_user_pointer(objects);
>  	execbuf.buffer_count = ibb->num_objects;
>  	execbuf.batch_len = end_offset;
>  	execbuf.rsvd1 = ibb->ctx;
> -- 
> 2.29.2

Ok, maybe these asserts will answer why sometimes we got SIGSEGV.
This would be real cornercase with malloc() so lets find out is it 
properly handled.

Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>

--
Zbigniew

> 
> _______________________________________________
> igt-dev mailing list
> igt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev