[Intel-gfx] [PATCH] drm/i915/gem: Add a check for object size for corner cases

[Chris Wilson]

Quoting Ram Moon, AnandX (2021-02-16 12:05:23)
> Hi Chris,
> 
> -----Original Message-----
> From: dri-devel <dri-devel-bounces at lists.freedesktop.org> On Behalf Of Chris Wilson
> Sent: Monday, February 15, 2021 6:10 PM
> To: Auld, Matthew <matthew.auld at intel.com>; Ram Moon, AnandX <anandx.ram.moon at intel.com>; Surendrakumar Upadhyay, TejaskumarX <tejaskumarx.surendrakumar.upadhyay at intel.com>; Ursulin, Tvrtko <tvrtko.ursulin at intel.com>; Jani Nikula <jani.nikula at linux.intel.com>; dri-devel at lists.freedesktop.org; intel-gfx at lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/gem: Add a check for object size for corner cases
> 
> Quoting Ram Moon, AnandX (2021-02-15 12:29:17)
> > Hi Chris,
> > 
> > -----Original Message-----
> > From: dri-devel <dri-devel-bounces at lists.freedesktop.org> On Behalf Of 
> > Chris Wilson
> > Sent: Wednesday, February 10, 2021 4:15 PM
> > To: Ram Moon, AnandX <anandx.ram.moon at intel.com>; Jani Nikula 
> > <jani.nikula at linux.intel.com>; Auld, Matthew <matthew.auld at intel.com>; 
> > Surendrakumar Upadhyay, TejaskumarX 
> > <tejaskumarx.surendrakumar.upadhyay at intel.com>; Ursulin, Tvrtko 
> > <tvrtko.ursulin at intel.com>; dri-devel at lists.freedesktop.org; 
> > intel-gfx at lists.freedesktop.org
> > Cc: Ram Moon, AnandX <anandx.ram.moon at intel.com>
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/gem: Add a check for object 
> > size for corner cases
> > 
> > Quoting Anand Moon (2021-02-10 07:59:29)
> > > Add check for object size to return appropriate error -E2BIG or 
> > > -EINVAL to avoid WARM_ON and successful return for some testcase.
> > 
> > No. You miss the point of having those warnings. We need to inspect the code to remove the last remaining "int pagenum", and then we can remove the GEM_WARN_ON((sz) >> PAGE_SHIFT > INT_MAX). These are not emitted for users, only for us to motivate us into finally fixing the code.
> > -Chris
> > 
> > Yes, I got your point these check are meant to catch up integer overflow.
> > 
> > I have check with the IGT testcase case  _gem_create_ and 
> > _gem_userptr_blits_ which fails pass size *-1ull << 32*  left shift 
> > and *0~* which leads to integer overflow ie  _negative_ size passed to create  i915_gem_create via ioctl  this leads to GM_WARN_ON.
> > 
> > Can we drop these testcase so that we don't break the kernel ?
> 
> The kernel rejects the ioctl with the expected errno. We leave a warning purely for the benefit of CI, only when compiled for debugging and not by default, so that we have a persistent reminder to do the code review.
> What's broken?
> -Chris
> 
> All though the testcase return with appropriate error we observe kernel taint see below.

Which is an intentional taint added for CI so that we get a warning and
a visible bug so that we can allocate resources to _fix_ the underlying
problems in the code.
-Chris