Quoting Ram Moon, AnandX (2021-02-16 12:05:23)
Hi Chris,
-----Original Message----- From: dri-devel dri-devel-bounces@lists.freedesktop.org On Behalf Of Chris Wilson Sent: Monday, February 15, 2021 6:10 PM To: Auld, Matthew matthew.auld@intel.com; Ram Moon, AnandX anandx.ram.moon@intel.com; Surendrakumar Upadhyay, TejaskumarX tejaskumarx.surendrakumar.upadhyay@intel.com; Ursulin, Tvrtko tvrtko.ursulin@intel.com; Jani Nikula jani.nikula@linux.intel.com; dri-devel@lists.freedesktop.org; intel-gfx@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@lists.freedesktop.org On Behalf Of Chris Wilson Sent: Wednesday, February 10, 2021 4:15 PM To: Ram Moon, AnandX anandx.ram.moon@intel.com; Jani Nikula jani.nikula@linux.intel.com; Auld, Matthew matthew.auld@intel.com; Surendrakumar Upadhyay, TejaskumarX tejaskumarx.surendrakumar.upadhyay@intel.com; Ursulin, Tvrtko tvrtko.ursulin@intel.com; dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org Cc: Ram Moon, AnandX anandx.ram.moon@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