linux-sparse.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	mchehab@kernel.org, chris@chris-wilson.co.uk,
	matthew.auld@intel.com, thomas.hellstrom@linux.intel.com,
	jani.nikula@intel.com, nirmoy.das@intel.com, airlied@redhat.com,
	daniel@ffwll.ch, andi.shyti@linux.intel.com,
	andrzej.hajda@intel.com, keescook@chromium.org,
	mauro.chehab@linux.intel.com, linux@rasmusvillemoes.dk,
	vitor@massaru.org, dlatypov@google.com, ndesaulniers@google.com,
	trix@redhat.com, llvm@lists.linux.dev,
	linux-hardening@vger.kernel.org, linux-sparse@vger.kernel.org,
	nathan@kernel.org, gustavoars@kernel.org,
	luc.vanoostenryck@gmail.com
Subject: [PATCH v12 6/9] drm/i915: Check for integer truncation on the configuration of ttm place
Date: Mon, 26 Sep 2022 18:39:50 +0300	[thread overview]
Message-ID: <20220926153953.3836470-7-gwan-gyeong.mun@intel.com> (raw)
In-Reply-To: <20220926153953.3836470-1-gwan-gyeong.mun@intel.com>

There is an impedance mismatch between the first/last valid page
frame number of ttm place in unsigned and our memory/page accounting in
unsigned long.
As the object size is under the control of userspace, we have to be prudent
and catch the conversion errors.
To catch the implicit truncation as we switch from unsigned long to
unsigned, we use overflows_type check and report E2BIG or overflow_type
prior to the operation.

v3: Not to change execution inside a macro. (Mauro)
    Add safe_conversion_gem_bug_on() macro and remove temporal
    SAFE_CONVERSION() macro.
v4: Fix unhandled GEM_BUG_ON() macro call from safe_conversion_gem_bug_on()
v6: Fix to follow general use case for GEM_BUG_ON(). (Jani)
v7: Fix to use WARN_ON() macro where GEM_BUG_ON() macro was used. (Jani)
v8: Replace safe_conversion() with check_assign() (Kees)

Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Nirmoy Das <nirmoy.das@intel.com> (v2)
Reviewed-by: Mauro Carvalho Chehab <mchehab@kernel.org> (v3)
Reported-by: kernel test robot <lkp@intel.com>
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> (v5)
---
 drivers/gpu/drm/i915/gem/i915_gem_ttm.c |  6 +++---
 drivers/gpu/drm/i915/intel_region_ttm.c | 17 ++++++++++++++---
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index bf99d1f02bd9..e6bbc0f8b7e6 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -140,14 +140,14 @@ i915_ttm_place_from_region(const struct intel_memory_region *mr,
 	if (flags & I915_BO_ALLOC_CONTIGUOUS)
 		place->flags |= TTM_PL_FLAG_CONTIGUOUS;
 	if (offset != I915_BO_INVALID_OFFSET) {
-		place->fpfn = offset >> PAGE_SHIFT;
-		place->lpfn = place->fpfn + (size >> PAGE_SHIFT);
+		WARN_ON(check_assign(offset >> PAGE_SHIFT, &place->fpfn));
+		WARN_ON(check_assign(place->fpfn + (size >> PAGE_SHIFT), &place->lpfn));
 	} else if (mr->io_size && mr->io_size < mr->total) {
 		if (flags & I915_BO_ALLOC_GPU_ONLY) {
 			place->flags |= TTM_PL_FLAG_TOPDOWN;
 		} else {
 			place->fpfn = 0;
-			place->lpfn = mr->io_size >> PAGE_SHIFT;
+			WARN_ON(check_assign(mr->io_size >> PAGE_SHIFT, &place->lpfn));
 		}
 	}
 }
diff --git a/drivers/gpu/drm/i915/intel_region_ttm.c b/drivers/gpu/drm/i915/intel_region_ttm.c
index 575d67bc6ffe..37a964b20b36 100644
--- a/drivers/gpu/drm/i915/intel_region_ttm.c
+++ b/drivers/gpu/drm/i915/intel_region_ttm.c
@@ -209,14 +209,23 @@ intel_region_ttm_resource_alloc(struct intel_memory_region *mem,
 	if (flags & I915_BO_ALLOC_CONTIGUOUS)
 		place.flags |= TTM_PL_FLAG_CONTIGUOUS;
 	if (offset != I915_BO_INVALID_OFFSET) {
-		place.fpfn = offset >> PAGE_SHIFT;
-		place.lpfn = place.fpfn + (size >> PAGE_SHIFT);
+		if (WARN_ON(check_assign(offset >> PAGE_SHIFT, &place.fpfn))) {
+			ret = -E2BIG;
+			goto out;
+		}
+		if (WARN_ON(check_assign(place.fpfn + (size >> PAGE_SHIFT), &place.lpfn))) {
+			ret = -E2BIG;
+			goto out;
+		}
 	} else if (mem->io_size && mem->io_size < mem->total) {
 		if (flags & I915_BO_ALLOC_GPU_ONLY) {
 			place.flags |= TTM_PL_FLAG_TOPDOWN;
 		} else {
 			place.fpfn = 0;
-			place.lpfn = mem->io_size >> PAGE_SHIFT;
+			if (WARN_ON(check_assign(mem->io_size >> PAGE_SHIFT, &place.lpfn))) {
+				ret = -E2BIG;
+				goto out;
+			}
 		}
 	}
 
@@ -224,6 +233,8 @@ intel_region_ttm_resource_alloc(struct intel_memory_region *mem,
 	mock_bo.bdev = &mem->i915->bdev;
 
 	ret = man->func->alloc(man, &mock_bo, &place, &res);
+
+out:
 	if (ret == -ENOSPC)
 		ret = -ENXIO;
 	if (!ret)
-- 
2.37.1


  parent reply	other threads:[~2022-09-26 16:48 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-26 15:39 [PATCH v12 0/9] Fixes integer overflow or integer truncation issues in page lookups, ttm place configuration and scatterlist creation Gwan-gyeong Mun
2022-09-26 15:39 ` [PATCH v12 1/9] overflow: Allow mixed type arguments Gwan-gyeong Mun
2022-09-26 15:39 ` [PATCH v12 2/9] overflow: Introduce check_assign() and check_assign_user_ptr() Gwan-gyeong Mun
2022-09-26 15:39 ` [PATCH v12 3/9] overflow: Introduce overflows_type() and __castable_to_type() Gwan-gyeong Mun
2022-09-28  6:21   ` [Intel-gfx] " kernel test robot
2022-09-29  7:01   ` kernel test robot
2022-09-26 15:39 ` [PATCH v12 4/9] drm/i915/gem: Typecheck page lookups Gwan-gyeong Mun
2022-09-26 15:39 ` [PATCH v12 5/9] drm/i915: Check for integer truncation on scatterlist creation Gwan-gyeong Mun
2022-09-26 15:39 ` Gwan-gyeong Mun [this message]
2022-09-26 15:39 ` [PATCH v12 7/9] drm/i915: Check if the size is too big while creating shmem file Gwan-gyeong Mun
2022-09-26 15:39 ` [PATCH v12 8/9] drm/i915: Use error code as -E2BIG when the size of gem ttm object is too large Gwan-gyeong Mun
2022-09-26 15:39 ` [PATCH v12 9/9] drm/i915: Remove truncation warning for large objects Gwan-gyeong Mun

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220926153953.3836470-7-gwan-gyeong.mun@intel.com \
    --to=gwan-gyeong.mun@intel.com \
    --cc=airlied@redhat.com \
    --cc=andi.shyti@linux.intel.com \
    --cc=andrzej.hajda@intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=daniel@ffwll.ch \
    --cc=dlatypov@google.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gustavoars@kernel.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    --cc=keescook@chromium.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sparse@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=llvm@lists.linux.dev \
    --cc=luc.vanoostenryck@gmail.com \
    --cc=matthew.auld@intel.com \
    --cc=mauro.chehab@linux.intel.com \
    --cc=mchehab@kernel.org \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=nirmoy.das@intel.com \
    --cc=thomas.hellstrom@linux.intel.com \
    --cc=trix@redhat.com \
    --cc=vitor@massaru.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).