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

Daniel Vetter
2016-12-27T13:12:08Z

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