Re: [PATCH v4 25/38] drm: Add asserts to catch overflow in drm_mm_init() and drm_mm_init_scan()

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Dec 22, 2016 at 08:36:28AM +0000, Chris Wilson wrote:
> A simple assert to ensure that we don't overflow start + size when
> initialising the drm_mm, or its scanner.
> 
> In future, we may want to switch to tracking the value of ranges (rather
> than size) so that we can cover the full u64, for example like resource
> tracking.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_mm.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index e0419cf09bbb..b80305484124 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -729,6 +729,8 @@ void drm_mm_init_scan(struct drm_mm *mm,
>  		      u64 alignment,
>  		      unsigned long color)
>  {
> +	DRM_MM_BUG_ON(!size);

General thought: Do we/should we have testcases for these negative checks?
Adding a drm_mm_test_mode or similar that DRM_MM_BUG_ON checks before
calling BUG_ON, and using that in drm_mm_selftest.ko would make that work.

Maybe we could even do that somewhat generically in drm_selftect code.
Since we can't continue without causing real havoc a possible solution
would be to just exit the current thread. And provdide a drm_selftest.c
wrapper which sets this option, runs the testcode in a separate thread and
then checks that we've exited through DRM_MM_BUG_ON.

Just as an idea really.
-Daniel

> +
>  	mm->scan_color = color;
>  	mm->scan_alignment = alignment;
>  	mm->scan_size = size;
> @@ -764,6 +766,9 @@ void drm_mm_init_scan_with_range(struct drm_mm *mm,
>  				 u64 start,
>  				 u64 end)
>  {
> +	DRM_MM_BUG_ON(start >= end);
> +	DRM_MM_BUG_ON(!size || size > end - start);
> +
>  	mm->scan_color = color;
>  	mm->scan_alignment = alignment;
>  	mm->scan_size = size;
> @@ -882,6 +887,8 @@ EXPORT_SYMBOL(drm_mm_scan_remove_block);
>   */
>  void drm_mm_init(struct drm_mm *mm, u64 start, u64 size)
>  {
> +	DRM_MM_BUG_ON(start + size <= start);
> +
>  	INIT_LIST_HEAD(&mm->hole_stack);
>  	mm->scanned_blocks = 0;
>  
> -- 
> 2.11.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx