From patchwork Fri Sep 9 10:59:05 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gwan-gyeong Mun X-Patchwork-Id: 12971533 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id DF966C6FA82 for ; Fri, 9 Sep 2022 11:00:47 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B700810EC74; Fri, 9 Sep 2022 11:00:02 +0000 (UTC) Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id D67BE10EC69; Fri, 9 Sep 2022 10:59:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1662721190; x=1694257190; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=69FBMpVYFEUjFQCOfnxuvXYL4ONxLz7KIzDaR+QH+Wo=; b=ktoijDufVAP6/iyo4liV4LytUl55PUckyHz+E8vXEmz6RQ3I7EHQsTRx eQT4WZlXw8aduly96Jr71WC952xtzwO6ZDMyMKEydWc61OibFYfUdlDGK 9IL4pW1R1x9GrB2vfQgOjWwr2mTOlxlfnOYosFTWTehPuda+Lccce2gsP 1y6nyuyE8gQnhV/uFyB2vQV26OW16YHbw/nabrAPPrivL8dfPOSDNNtVc uQ+510IaCix4oUkIIL/VGPszLSaMYNTXqC41YKHO8NyTTc6+JjOgLCJBS 5uy6p8ksfInkRBf01wFVVHEDtT+l5Unu6GhOJfTMSimm8VkL0HAp86/I2 Q==; X-IronPort-AV: E=McAfee;i="6500,9779,10464"; a="359174653" X-IronPort-AV: E=Sophos;i="5.93,303,1654585200"; d="scan'208";a="359174653" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Sep 2022 03:59:41 -0700 X-IronPort-AV: E=Sophos;i="5.93,303,1654585200"; d="scan'208";a="704370301" Received: from clbarnes-mobl.amr.corp.intel.com (HELO paris.amr.corp.intel.com) ([10.254.42.92]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Sep 2022 03:59:37 -0700 From: Gwan-gyeong Mun To: intel-gfx@lists.freedesktop.org Date: Fri, 9 Sep 2022 19:59:05 +0900 Message-Id: <20220909105913.752049-2-gwan-gyeong.mun@intel.com> X-Mailer: git-send-email 2.37.1 In-Reply-To: <20220909105913.752049-1-gwan-gyeong.mun@intel.com> References: <20220909105913.752049-1-gwan-gyeong.mun@intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH v10 1/9] overflow: Allow mixed type arguments X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: thomas.hellstrom@linux.intel.com, keescook@chromium.org, jani.nikula@intel.com, ndesaulniers@google.com, dlatypov@google.com, linux@rasmusvillemoes.dk, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, chris@chris-wilson.co.uk, andrzej.hajda@intel.com, matthew.auld@intel.com, daniel@ffwll.ch, airlied@redhat.com, mchehab@kernel.org, vitor@massaru.org, nirmoy.das@intel.com Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" From: Kees Cook When the check_[op]_overflow() helpers were introduced, all arguments were required to be the same type to make the fallback macros simpler. However, now that the fallback macros have been removed[1], it is fine to allow mixed types, which makes using the helpers much more useful, as they can be used to test for type-based overflows (e.g. adding two large ints but storing into a u8), as would be handy in the drm core[2]. Remove the restriction, and add additional self-tests that exercise some of the mixed-type overflow cases, and double-check for accidental macro side-effects. [1] https://git.kernel.org/linus/4eb6bd55cfb22ffc20652732340c4962f3ac9a91 [2] https://lore.kernel.org/lkml/20220824084514.2261614-2-gwan-gyeong.mun@intel.com Cc: Rasmus Villemoes Cc: Gwan-gyeong Mun Cc: Andrzej Hajda Cc: "Gustavo A. R. Silva" Cc: Nick Desaulniers Cc: linux-hardening@vger.kernel.org Signed-off-by: Kees Cook Reviewed-by: Andrzej Hajda --- include/linux/overflow.h | 72 ++++++++++++++++------------ lib/overflow_kunit.c | 101 ++++++++++++++++++++++++++++----------- 2 files changed, 113 insertions(+), 60 deletions(-) diff --git a/include/linux/overflow.h b/include/linux/overflow.h index 0eb3b192f07a..19dfdd74835e 100644 --- a/include/linux/overflow.h +++ b/include/linux/overflow.h @@ -51,40 +51,50 @@ static inline bool __must_check __must_check_overflow(bool overflow) return unlikely(overflow); } -/* - * For simplicity and code hygiene, the fallback code below insists on - * a, b and *d having the same type (similar to the min() and max() - * macros), whereas gcc's type-generic overflow checkers accept - * different types. Hence we don't just make check_add_overflow an - * alias for __builtin_add_overflow, but add type checks similar to - * below. +/** check_add_overflow() - Calculate addition with overflow checking + * + * @a: first addend + * @b: second addend + * @d: pointer to store sum + * + * Returns 0 on success. + * + * *@d holds the results of the attempted addition, but is not considered + * "safe for use" on a non-zero return value, which indicates that the + * sum has overflowed or been truncated. */ -#define check_add_overflow(a, b, d) __must_check_overflow(({ \ - typeof(a) __a = (a); \ - typeof(b) __b = (b); \ - typeof(d) __d = (d); \ - (void) (&__a == &__b); \ - (void) (&__a == __d); \ - __builtin_add_overflow(__a, __b, __d); \ -})) +#define check_add_overflow(a, b, d) \ + __must_check_overflow(__builtin_add_overflow(a, b, d)) -#define check_sub_overflow(a, b, d) __must_check_overflow(({ \ - typeof(a) __a = (a); \ - typeof(b) __b = (b); \ - typeof(d) __d = (d); \ - (void) (&__a == &__b); \ - (void) (&__a == __d); \ - __builtin_sub_overflow(__a, __b, __d); \ -})) +/** check_sub_overflow() - Calculate subtraction with overflow checking + * + * @a: minuend; value to subtract from + * @b: subtrahend; value to subtract from @a + * @d: pointer to store difference + * + * Returns 0 on success. + * + * *@d holds the results of the attempted subtraction, but is not considered + * "safe for use" on a non-zero return value, which indicates that the + * difference has underflowed or been truncated. + */ +#define check_sub_overflow(a, b, d) \ + __must_check_overflow(__builtin_sub_overflow(a, b, d)) -#define check_mul_overflow(a, b, d) __must_check_overflow(({ \ - typeof(a) __a = (a); \ - typeof(b) __b = (b); \ - typeof(d) __d = (d); \ - (void) (&__a == &__b); \ - (void) (&__a == __d); \ - __builtin_mul_overflow(__a, __b, __d); \ -})) +/** check_mul_overflow() - Calculate multiplication with overflow checking + * + * @a: first factor + * @b: second factor + * @d: pointer to store product + * + * Returns 0 on success. + * + * *@d holds the results of the attempted multiplication, but is not + * considered "safe for use" on a non-zero return value, which indicates + * that the product has overflowed or been truncated. + */ +#define check_mul_overflow(a, b, d) \ + __must_check_overflow(__builtin_mul_overflow(a, b, d)) /** check_shl_overflow() - Calculate a left-shifted value and check overflow * diff --git a/lib/overflow_kunit.c b/lib/overflow_kunit.c index 7e3e43679b73..0d98c9bc75da 100644 --- a/lib/overflow_kunit.c +++ b/lib/overflow_kunit.c @@ -16,12 +16,15 @@ #include #include -#define DEFINE_TEST_ARRAY(t) \ - static const struct test_ ## t { \ - t a, b; \ - t sum, diff, prod; \ - bool s_of, d_of, p_of; \ - } t ## _tests[] +#define DEFINE_TEST_ARRAY_TYPED(t1, t2, t) \ + static const struct test_ ## t1 ## _ ## t2 ## __ ## t { \ + t1 a; \ + t2 b; \ + t sum, diff, prod; \ + bool s_of, d_of, p_of; \ + } t1 ## _ ## t2 ## __ ## t ## _tests[] + +#define DEFINE_TEST_ARRAY(t) DEFINE_TEST_ARRAY_TYPED(t, t, t) DEFINE_TEST_ARRAY(u8) = { {0, 0, 0, 0, 0, false, false, false}, @@ -222,21 +225,27 @@ DEFINE_TEST_ARRAY(s64) = { }; #endif -#define check_one_op(t, fmt, op, sym, a, b, r, of) do { \ - t _r; \ - bool _of; \ - \ - _of = check_ ## op ## _overflow(a, b, &_r); \ - KUNIT_EXPECT_EQ_MSG(test, _of, of, \ +#define check_one_op(t, fmt, op, sym, a, b, r, of) do { \ + int _a_orig = a, _a_bump = a + 1; \ + int _b_orig = b, _b_bump = b + 1; \ + bool _of; \ + t _r; \ + \ + _of = check_ ## op ## _overflow(a, b, &_r); \ + KUNIT_EXPECT_EQ_MSG(test, _of, of, \ "expected "fmt" "sym" "fmt" to%s overflow (type %s)\n", \ - a, b, of ? "" : " not", #t); \ - KUNIT_EXPECT_EQ_MSG(test, _r, r, \ + a, b, of ? "" : " not", #t); \ + KUNIT_EXPECT_EQ_MSG(test, _r, r, \ "expected "fmt" "sym" "fmt" == "fmt", got "fmt" (type %s)\n", \ - a, b, r, _r, #t); \ + a, b, r, _r, #t); \ + /* Check for internal macro side-effects. */ \ + _of = check_ ## op ## _overflow(_a_orig++, _b_orig++, &_r); \ + KUNIT_EXPECT_EQ_MSG(test, _a_orig, _a_bump, "Unexpected " #op " macro side-effect!\n"); \ + KUNIT_EXPECT_EQ_MSG(test, _b_orig, _b_bump, "Unexpected " #op " macro side-effect!\n"); \ } while (0) -#define DEFINE_TEST_FUNC(t, fmt) \ -static void do_test_ ## t(struct kunit *test, const struct test_ ## t *p) \ +#define DEFINE_TEST_FUNC_TYPED(n, t, fmt) \ +static void do_test_ ## n(struct kunit *test, const struct test_ ## n *p) \ { \ check_one_op(t, fmt, add, "+", p->a, p->b, p->sum, p->s_of); \ check_one_op(t, fmt, add, "+", p->b, p->a, p->sum, p->s_of); \ @@ -245,15 +254,18 @@ static void do_test_ ## t(struct kunit *test, const struct test_ ## t *p) \ check_one_op(t, fmt, mul, "*", p->b, p->a, p->prod, p->p_of); \ } \ \ -static void t ## _overflow_test(struct kunit *test) { \ +static void n ## _overflow_test(struct kunit *test) { \ unsigned i; \ \ - for (i = 0; i < ARRAY_SIZE(t ## _tests); ++i) \ - do_test_ ## t(test, &t ## _tests[i]); \ + for (i = 0; i < ARRAY_SIZE(n ## _tests); ++i) \ + do_test_ ## n(test, &n ## _tests[i]); \ kunit_info(test, "%zu %s arithmetic tests finished\n", \ - ARRAY_SIZE(t ## _tests), #t); \ + ARRAY_SIZE(n ## _tests), #n); \ } +#define DEFINE_TEST_FUNC(t, fmt) \ + DEFINE_TEST_FUNC_TYPED(t ## _ ## t ## __ ## t, t, fmt) + DEFINE_TEST_FUNC(u8, "%d"); DEFINE_TEST_FUNC(s8, "%d"); DEFINE_TEST_FUNC(u16, "%d"); @@ -265,6 +277,33 @@ DEFINE_TEST_FUNC(u64, "%llu"); DEFINE_TEST_FUNC(s64, "%lld"); #endif +DEFINE_TEST_ARRAY_TYPED(u32, u32, u8) = { + {0, 0, 0, 0, 0, false, false, false}, + {U8_MAX, 2, 1, U8_MAX - 2, U8_MAX - 1, true, false, true}, + {U8_MAX + 1, 0, 0, 0, 0, true, true, false}, +}; +DEFINE_TEST_FUNC_TYPED(u32_u32__u8, u8, "%d"); + +DEFINE_TEST_ARRAY_TYPED(u32, u32, int) = { + {0, 0, 0, 0, 0, false, false, false}, + {U32_MAX, 0, -1, -1, 0, true, true, false}, +}; +DEFINE_TEST_FUNC_TYPED(u32_u32__int, int, "%d"); + +DEFINE_TEST_ARRAY_TYPED(u8, u8, int) = { + {0, 0, 0, 0, 0, false, false, false}, + {U8_MAX, U8_MAX, 2 * U8_MAX, 0, U8_MAX * U8_MAX, false, false, false}, + {1, 2, 3, -1, 2, false, false, false}, +}; +DEFINE_TEST_FUNC_TYPED(u8_u8__int, int, "%d"); + +DEFINE_TEST_ARRAY_TYPED(int, int, u8) = { + {0, 0, 0, 0, 0, false, false, false}, + {1, 2, 3, U8_MAX, 2, false, true, false}, + {-1, 0, U8_MAX, U8_MAX, 0, true, true, false}, +}; +DEFINE_TEST_FUNC_TYPED(int_int__u8, u8, "%d"); + static void overflow_shift_test(struct kunit *test) { int count = 0; @@ -649,17 +688,21 @@ static void overflow_size_helpers_test(struct kunit *test) } static struct kunit_case overflow_test_cases[] = { - KUNIT_CASE(u8_overflow_test), - KUNIT_CASE(s8_overflow_test), - KUNIT_CASE(u16_overflow_test), - KUNIT_CASE(s16_overflow_test), - KUNIT_CASE(u32_overflow_test), - KUNIT_CASE(s32_overflow_test), + KUNIT_CASE(u8_u8__u8_overflow_test), + KUNIT_CASE(s8_s8__s8_overflow_test), + KUNIT_CASE(u16_u16__u16_overflow_test), + KUNIT_CASE(s16_s16__s16_overflow_test), + KUNIT_CASE(u32_u32__u32_overflow_test), + KUNIT_CASE(s32_s32__s32_overflow_test), /* Clang 13 and earlier generate unwanted libcalls on 32-bit. */ #if BITS_PER_LONG == 64 - KUNIT_CASE(u64_overflow_test), - KUNIT_CASE(s64_overflow_test), + KUNIT_CASE(u64_u64__u64_overflow_test), + KUNIT_CASE(s64_s64__s64_overflow_test), #endif + KUNIT_CASE(u32_u32__u8_overflow_test), + KUNIT_CASE(u32_u32__int_overflow_test), + KUNIT_CASE(u8_u8__int_overflow_test), + KUNIT_CASE(int_int__u8_overflow_test), KUNIT_CASE(overflow_shift_test), KUNIT_CASE(overflow_allocation_test), KUNIT_CASE(overflow_size_helpers_test), From patchwork Fri Sep 9 10:59:06 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Gwan-gyeong Mun X-Patchwork-Id: 12971532 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 3EE1EECAAD3 for ; Fri, 9 Sep 2022 11:00:44 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 67EFB10EC6E; Fri, 9 Sep 2022 11:00:01 +0000 (UTC) Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 18D2910EC68; Fri, 9 Sep 2022 10:59:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1662721191; x=1694257191; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=BqnvQZZmqlppIrLqxR7AWPt8tNJLCHPb5qBykGHKpmQ=; b=EbepyTpaQ1CXUGORcwioYAIxFwjFUTnsc9bwpFq05uv841Zb8F3BDlpa XRLvaipvDHZxoaoGHUNsu6LmB+y4JO8j6HI14OMOpYlgxhKnPPYVj95FD 9ukv8236rVdcEqO0kZboDm8GRZhnjCDCEm0hSc5MvddemYV0EfCeMXQ/6 7ZacikiR5c1E2CgfXB48lv4Wv6L2B9TqYnjg3nUZJqOCKze4FujSASmXQ bgQMbm/WvBkV1iQQLeGaVplWUSemPGJLoe6ek1AzSQnt2eKJlBEy43oqf enoH0aGlqNM0oL8UR6B3/xMbGYR8Vgf9Qqnk5Z8JjLjPcxEy2kMWQJtbw w==; X-IronPort-AV: E=McAfee;i="6500,9779,10464"; a="359174658" X-IronPort-AV: E=Sophos;i="5.93,303,1654585200"; d="scan'208";a="359174658" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Sep 2022 03:59:44 -0700 X-IronPort-AV: E=Sophos;i="5.93,303,1654585200"; d="scan'208";a="704370317" Received: from clbarnes-mobl.amr.corp.intel.com (HELO paris.amr.corp.intel.com) ([10.254.42.92]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Sep 2022 03:59:41 -0700 From: Gwan-gyeong Mun To: intel-gfx@lists.freedesktop.org Date: Fri, 9 Sep 2022 19:59:06 +0900 Message-Id: <20220909105913.752049-3-gwan-gyeong.mun@intel.com> X-Mailer: git-send-email 2.37.1 In-Reply-To: <20220909105913.752049-1-gwan-gyeong.mun@intel.com> References: <20220909105913.752049-1-gwan-gyeong.mun@intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH v10 2/9] overflow: Move and add few utility macros into overflow X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: thomas.hellstrom@linux.intel.com, keescook@chromium.org, jani.nikula@intel.com, ndesaulniers@google.com, dlatypov@google.com, linux@rasmusvillemoes.dk, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, chris@chris-wilson.co.uk, andrzej.hajda@intel.com, matthew.auld@intel.com, daniel@ffwll.ch, airlied@redhat.com, mchehab@kernel.org, vitor@massaru.org, nirmoy.das@intel.com Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" It moves overflows_type utility macro into overflow header from i915_utils header. The overflows_type can be used to catch the truncaion (overflow) between different data types. And it adds check_assign() macro which performs an assigning source value into destination pointer along with an overflow check. overflow_type macro has been improved to handle the different data types between source and destination by check_add_overflow macro. It also adds check_assign_user_ptr macro which performs an assigning source value into destination pointer type variable along with an overflow check. If an explicit overflow check is required while assigning, check_assign_user_ptr() can be used to assign integers into pointers along with an overflow check. v3: Add is_type_unsigned() macro (Mauro) Modify overflows_type() macro to consider signed data types (Mauro) Fix the problem that safe_conversion() macro always returns true v4: Fix kernel-doc markups v6: Move macro addition location so that it can be used by other than drm subsystem (Jani, Mauro, Andi) Change is_type_unsigned to is_unsigned_type to have the same name form as is_signed_type macro v8: Add check_assign() and remove safe_conversion() (Kees) Fix overflows_type() to use gcc's built-in overflow function (Andrzej) Add overflows_ptr() to allow overflow checking when assigning a value into a pointer variable (G.G.) v9: Fix overflows_type() to use __builtin_add_overflow() instead of __builtin_add_overflow_p() (Andrzej) Fix overflows_ptr() to use overflows_type() with the unsigned long type (Andrzej) v10: Remove a redundant type checking for a pointer. (Andrzej) Use updated check_add_overflow macro instead of __builtin_add_overflow (G.G) Add check_assign_user_ptr() macro and drop overflows_ptr() macro(Kees) Signed-off-by: Gwan-gyeong Mun Cc: Thomas Hellström Cc: Matthew Auld Cc: Nirmoy Das Cc: Jani Nikula Cc: Andi Shyti Cc: Andrzej Hajda Cc: Mauro Carvalho Chehab Cc: Kees Cook Reviewed-by: Mauro Carvalho Chehab (v5) Reviewed-by: Andrzej Hajda (v9) Acked-by: Kees Cook Reported-by: kernel test robot --- drivers/gpu/drm/i915/i915_user_extensions.c | 6 +- drivers/gpu/drm/i915/i915_utils.h | 5 +- include/linux/overflow.h | 64 +++++++++++++++++++++ 3 files changed, 68 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_user_extensions.c b/drivers/gpu/drm/i915/i915_user_extensions.c index c822d0aafd2d..80ec8390b0d8 100644 --- a/drivers/gpu/drm/i915/i915_user_extensions.c +++ b/drivers/gpu/drm/i915/i915_user_extensions.c @@ -50,11 +50,11 @@ int i915_user_extensions(struct i915_user_extension __user *ext, if (err) return err; - if (get_user(next, &ext->next_extension) || - overflows_type(next, ext)) + if (get_user(next, &ext->next_extension)) return -EFAULT; - ext = u64_to_user_ptr(next); + if (check_assign_user_ptr(next, ext)) + return -EFAULT; } return 0; diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h index 6c14d13364bf..efd3d69b78f7 100644 --- a/drivers/gpu/drm/i915/i915_utils.h +++ b/drivers/gpu/drm/i915/i915_utils.h @@ -32,6 +32,7 @@ #include #include #include +#include #ifdef CONFIG_X86 #include @@ -111,10 +112,6 @@ bool i915_error_injected(void); #define range_overflows_end_t(type, start, size, max) \ range_overflows_end((type)(start), (type)(size), (type)(max)) -/* Note we don't consider signbits :| */ -#define overflows_type(x, T) \ - (sizeof(x) > sizeof(T) && (x) >> BITS_PER_TYPE(T)) - #define ptr_mask_bits(ptr, n) ({ \ unsigned long __v = (unsigned long)(ptr); \ (typeof(ptr))(__v & -BIT(n)); \ diff --git a/include/linux/overflow.h b/include/linux/overflow.h index 19dfdd74835e..9e8fc8f03e7a 100644 --- a/include/linux/overflow.h +++ b/include/linux/overflow.h @@ -5,6 +5,7 @@ #include #include #include +#include /* * We need to compute the minimum and maximum values representable in a given @@ -127,6 +128,69 @@ static inline bool __must_check __must_check_overflow(bool overflow) (*_d >> _to_shift) != _a); \ })) +/** + * check_assign - perform an assigning source value into destination pointer + * along with an overflow check. + * + * @value: source value + * @ptr: Destination pointer address + * + * Returns: + * If the value would overflow the destination, it returns true. If not return + * false. When overflow does not occur, the assigning into destination from + * value succeeds. It follows the return policy as other check_*_overflow() + * functions return non-zero as a failure. + */ +#define check_assign(value, ptr) __must_check_overflow(({ \ + check_add_overflow(0, value, ptr); \ +})) + +/** + * check_assign_user_ptr - perform an assigning source value into destination + * pointer type variable along with an overflow check + * + * @value: source value; a source value is expected to have a value of a size + * that can be stored in a pointer-type variable. + * @ptr: destination pointer type variable + * + * u64_to_user_ptr can be used in the kernel to avoid warnings about integers + * and pointers of different sizes. But u64_to_user_ptr is not performing the + * checking of overflow. If you need an explicit overflow check while + * assigning, check_assign_user_ptr() can be used to assign integers into + * pointers along with an overflow check. If ptr is not a pointer type, + * a warning message outputs while compiling. + * + * Returns: + * If the value would overflow the destination, it returns true. If not return + * false. When overflow does not occur, the assigning into ptr from value + * succeeds. It follows the return policy as other check_*_overflow() functions + * return non-zero as a failure. + */ +#define check_assign_user_ptr(value, ptr) __must_check_overflow(({ \ + uintptr_t kptr; \ + check_assign(value, &kptr) ? 1 : (({ ptr = (void * __user)kptr; }), 0); \ +})) + +/** + * overflows_type - helper for checking the overflows between data types or + * values + * + * @x: source value or data type for overflow check + * @T: destination value or data type for overflow check + * + * It compares the values or data type between the first and second argument to + * check whether overflow can occur when assigning the first argument to the + * variable of the second argument. Source and Destination can be different data + * types. + * + * Returns: + * True if overflow can occur, false otherwise. + */ +#define overflows_type(x, T) __must_check_overflow(({ \ + typeof(T) v = 0; \ + check_add_overflow((x), v, &v); \ +})) + /** * size_mul() - Calculate size_t multiplication with saturation at SIZE_MAX * From patchwork Fri Sep 9 10:59:07 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Gwan-gyeong Mun X-Patchwork-Id: 12971534 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 73EA8C6FA86 for ; Fri, 9 Sep 2022 11:00:49 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 230F410EC7A; Fri, 9 Sep 2022 11:00:04 +0000 (UTC) Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2738E10EC6A; Fri, 9 Sep 2022 10:59:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1662721191; x=1694257191; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=sw25/VEVNzYbUpj2DtWImbIBzsU8rdEcHgaL9vecVhE=; b=imY5ckQ6J1W1c5oH3/RBA6JWA7LW6AiYVNsvEL9xWPtLNp/ktJo2M6BE oOhfvAp4wx9Y7O6kT4z1XXjO5OyUGVsyX5OwQZDMZwG48doUO6R2WFkHk UpBUuI9Nt8RN/CnfqS0FiFK1CP8QYDH9XwQwjiTy1gN+rMYlDBfXwC6pk TMG/aYpeOLUhlt8TBZKcteGGWoYDs/7S6+ayu2TSHbf053csJvBsBZjQY 5M3jmi6ojiiBMxAGF+5wo9prq6izcyGsD00/JJE1pAKbW6SzGF9HqUuvG 0QKcsIOBm3WiquEWjCIE4z9M9GeYnrlvS9EsYNgDr0D4oBC4SR4B5kk4L w==; X-IronPort-AV: E=McAfee;i="6500,9779,10464"; a="359174665" X-IronPort-AV: E=Sophos;i="5.93,303,1654585200"; d="scan'208";a="359174665" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Sep 2022 03:59:48 -0700 X-IronPort-AV: E=Sophos;i="5.93,303,1654585200"; d="scan'208";a="704370341" Received: from clbarnes-mobl.amr.corp.intel.com (HELO paris.amr.corp.intel.com) ([10.254.42.92]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Sep 2022 03:59:44 -0700 From: Gwan-gyeong Mun To: intel-gfx@lists.freedesktop.org Date: Fri, 9 Sep 2022 19:59:07 +0900 Message-Id: <20220909105913.752049-4-gwan-gyeong.mun@intel.com> X-Mailer: git-send-email 2.37.1 In-Reply-To: <20220909105913.752049-1-gwan-gyeong.mun@intel.com> References: <20220909105913.752049-1-gwan-gyeong.mun@intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH v10 3/9] compiler_types.h: Add assert_type to catch type mis-match while compiling X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: thomas.hellstrom@linux.intel.com, keescook@chromium.org, jani.nikula@intel.com, ndesaulniers@google.com, dlatypov@google.com, linux@rasmusvillemoes.dk, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, chris@chris-wilson.co.uk, andrzej.hajda@intel.com, matthew.auld@intel.com, daniel@ffwll.ch, airlied@redhat.com, mchehab@kernel.org, vitor@massaru.org, nirmoy.das@intel.com Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" It adds assert_type and assert_typable macros to catch type mis-match while compiling. The existing typecheck() macro outputs build warnings, but the newly added assert_type() macro uses the _Static_assert() keyword (which is introduced in C11) to generate a build break when the types are different and can be used to detect explicit build errors. Unlike the assert_type() macro, assert_typable() macro allows a constant value as the second argument. Suggested-by: Kees Cook Signed-off-by: Gwan-gyeong Mun Cc: Thomas Hellström Cc: Matthew Auld Cc: Nirmoy Das Cc: Jani Nikula Cc: Andi Shyti Cc: Mauro Carvalho Chehab Cc: Andrzej Hajda Cc: Kees Cook --- include/linux/compiler_types.h | 39 ++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h index 4f2a819fd60a..19cc125918bb 100644 --- a/include/linux/compiler_types.h +++ b/include/linux/compiler_types.h @@ -294,6 +294,45 @@ struct ftrace_likely_data { /* Are two types/vars the same type (ignoring qualifiers)? */ #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b)) +/** + * assert_type - break compile if the first argument's data type and the second + * argument's data type are not the same + * + * @t1: data type or variable + * @t2: data type or variable + * + * The first and second arguments can be data types or variables or mixed (the + * first argument is the data type and the second argument is variable or vice + * versa). It determines whether the first argument's data type and the second + * argument's data type are the same while compiling, and it breaks compile if + * the two types are not the same. + * See also assert_typable(). + */ +#define assert_type(t1, t2) _Static_assert(__same_type(t1, t2)) + +/** + * assert_typable - break compile if the first argument's data type and the + * second argument's data type are not the same + * + * @t: data type or variable + * @n: data type or variable or constant value + * + * The first and second arguments can be data types or variables or mixed (the + * first argument is the data type and the second argument is variable or vice + * versa). Unlike the assert_type() macro, this macro allows a constant value + * as the second argument. And if the second argument is a constant value, it + * always passes. And it doesn't mean that the types are explicitly the same. + * When a constant value is used as the second argument, if you need an + * overflow check when assigning a constant value to a variable of the type of + * the first argument, you can use the overflows_type() macro. When a constant + * value is not used as a second argument, it determines whether the first + * argument's data type and the second argument's data type are the same while + * compiling, and it breaks compile if the two types are not the same. + * See also assert_type() and overflows_type(). + */ +#define assert_typable(t, n) _Static_assert(__builtin_constant_p(n) || \ + __same_type(t, typeof(n))) + /* * __unqual_scalar_typeof(x) - Declare an unqualified scalar type, leaving * non-scalar types unchanged. From patchwork Fri Sep 9 10:59:08 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Gwan-gyeong Mun X-Patchwork-Id: 12971531 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 00502ECAAA1 for ; Fri, 9 Sep 2022 11:00:39 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id AB30610EC78; Fri, 9 Sep 2022 11:00:00 +0000 (UTC) Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9AD5910EC6A; Fri, 9 Sep 2022 10:59:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1662721193; x=1694257193; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=DB7grR/VrJPBqXIx4ea3yHUHHS/zx3MTSk1qzs8bxpo=; b=VVyl63oAuTWoA/8UMPdfoKHWLWkBz0KOlTdGEVjCAPAm2h4yss8nIpzJ I9UO2c1et1p5/MxhjcnTeIa0G9u7gIQ9Jspwh3PWamwYaqiwqnuW9Dxxl nc9VJOpjDbn/GnBSlPGJH4PNQ97s5O/i2cUJJrviMhTA/sONEMdUmD5Jx tECD03LBnOOROmhwtSNuuey4RNy1EG10WkBrp9gbMiA53TJGdLxloptYK l1AakLo25OC6om/nvqzBwnSp0bo7wF9RZsxCzEOITB0a0UJgM/JCD5gYm 612HaaA7bF7BNASPnzoSBzqM+CBbAUqAuINCvY1mWpDeIva9AebcjL6gJ g==; X-IronPort-AV: E=McAfee;i="6500,9779,10464"; a="359174678" X-IronPort-AV: E=Sophos;i="5.93,303,1654585200"; d="scan'208";a="359174678" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Sep 2022 03:59:52 -0700 X-IronPort-AV: E=Sophos;i="5.93,303,1654585200"; d="scan'208";a="704370360" Received: from clbarnes-mobl.amr.corp.intel.com (HELO paris.amr.corp.intel.com) ([10.254.42.92]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Sep 2022 03:59:48 -0700 From: Gwan-gyeong Mun To: intel-gfx@lists.freedesktop.org Date: Fri, 9 Sep 2022 19:59:08 +0900 Message-Id: <20220909105913.752049-5-gwan-gyeong.mun@intel.com> X-Mailer: git-send-email 2.37.1 In-Reply-To: <20220909105913.752049-1-gwan-gyeong.mun@intel.com> References: <20220909105913.752049-1-gwan-gyeong.mun@intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH v10 4/9] drm/i915/gem: Typecheck page lookups X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: thomas.hellstrom@linux.intel.com, keescook@chromium.org, jani.nikula@intel.com, ndesaulniers@google.com, dlatypov@google.com, linux@rasmusvillemoes.dk, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, chris@chris-wilson.co.uk, andrzej.hajda@intel.com, matthew.auld@intel.com, daniel@ffwll.ch, airlied@redhat.com, mchehab@kernel.org, vitor@massaru.org, nirmoy.das@intel.com Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" From: Chris Wilson We need to check that we avoid integer overflows when looking up a page, and so fix all the instances where we have mistakenly used a plain integer instead of a more suitable long. Be pedantic and add integer typechecking to the lookup so that we can be sure that we are safe. And it also uses pgoff_t as our page lookups must remain compatible with the page cache, pgoff_t is currently exactly unsigned long. v2: Move added i915_utils's macro into drm_util header (Jani N) v3: Make not use the same macro name on a function. (Mauro) For kernel-doc, macros and functions are handled in the same namespace, the same macro name on a function prevents ever adding documentation for it. v4: Add kernel-doc markups to the kAPI functions and macros (Mauoro) v5: Fix an alignment to match open parenthesis v6: Rebase v10: Use assert_typable instead of exactly_pgoff_t() macro. (Kees) Signed-off-by: Chris Wilson Signed-off-by: Gwan-gyeong Mun Cc: Tvrtko Ursulin Cc: Matthew Auld Cc: Thomas Hellström Cc: Kees Cook Reviewed-by: Nirmoy Das (v2) Reviewed-by: Mauro Carvalho Chehab (v3) Reviewed-by: Andrzej Hajda (v5) --- drivers/gpu/drm/i915/gem/i915_gem_object.c | 7 +- drivers/gpu/drm/i915/gem/i915_gem_object.h | 293 ++++++++++++++++-- drivers/gpu/drm/i915/gem/i915_gem_pages.c | 27 +- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +- .../drm/i915/gem/selftests/i915_gem_context.c | 12 +- .../drm/i915/gem/selftests/i915_gem_mman.c | 8 +- .../drm/i915/gem/selftests/i915_gem_object.c | 8 +- drivers/gpu/drm/i915/i915_gem.c | 18 +- drivers/gpu/drm/i915/i915_utils.h | 1 + drivers/gpu/drm/i915/i915_vma.c | 8 +- 10 files changed, 323 insertions(+), 61 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c index 85482a04d158..22a8c10eccec 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -413,10 +413,11 @@ void __i915_gem_object_invalidate_frontbuffer(struct drm_i915_gem_object *obj, static void i915_gem_object_read_from_page_kmap(struct drm_i915_gem_object *obj, u64 offset, void *dst, int size) { + pgoff_t idx = offset >> PAGE_SHIFT; void *src_map; void *src_ptr; - src_map = kmap_atomic(i915_gem_object_get_page(obj, offset >> PAGE_SHIFT)); + src_map = kmap_atomic(i915_gem_object_get_page(obj, idx)); src_ptr = src_map + offset_in_page(offset); if (!(obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_READ)) @@ -429,9 +430,10 @@ i915_gem_object_read_from_page_kmap(struct drm_i915_gem_object *obj, u64 offset, static void i915_gem_object_read_from_page_iomap(struct drm_i915_gem_object *obj, u64 offset, void *dst, int size) { + pgoff_t idx = offset >> PAGE_SHIFT; + dma_addr_t dma = i915_gem_object_get_dma_address(obj, idx); void __iomem *src_map; void __iomem *src_ptr; - dma_addr_t dma = i915_gem_object_get_dma_address(obj, offset >> PAGE_SHIFT); src_map = io_mapping_map_wc(&obj->mm.region->iomap, dma - obj->mm.region->region.start, @@ -460,6 +462,7 @@ i915_gem_object_read_from_page_iomap(struct drm_i915_gem_object *obj, u64 offset */ int i915_gem_object_read_from_page(struct drm_i915_gem_object *obj, u64 offset, void *dst, int size) { + GEM_BUG_ON(overflows_type(offset >> PAGE_SHIFT, pgoff_t)); GEM_BUG_ON(offset >= obj->base.size); GEM_BUG_ON(offset_in_page(offset) > PAGE_SIZE - size); GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj)); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h index 7317d4102955..8dd2b84468c8 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h @@ -27,8 +27,10 @@ enum intel_region_id; * spot such a local variable, please consider fixing! * * Aside from our own locals (for which we have no excuse!): - * - sg_table embeds unsigned int for num_pages - * - get_user_pages*() mixed ints with longs + * - sg_table embeds unsigned int for nents + * + * We can check for invalidly typed locals with typecheck(), see for example + * i915_gem_object_get_sg(). */ #define GEM_CHECK_SIZE_OVERFLOW(sz) \ GEM_WARN_ON((sz) >> PAGE_SHIFT > INT_MAX) @@ -363,44 +365,289 @@ i915_gem_object_get_tile_row_size(const struct drm_i915_gem_object *obj) int i915_gem_object_set_tiling(struct drm_i915_gem_object *obj, unsigned int tiling, unsigned int stride); +/** + * __i915_gem_object_page_iter_get_sg - helper to find the target scatterlist + * pointer and the target page position using pgoff_t n input argument and + * i915_gem_object_page_iter + * @obj: i915 GEM buffer object + * @iter: i915 GEM buffer object page iterator + * @n: page offset + * @offset: searched physical offset, + * it will be used for returning physical page offset value + * + * Context: Takes and releases the mutex lock of the i915_gem_object_page_iter. + * Takes and releases the RCU lock to search the radix_tree of + * i915_gem_object_page_iter. + * + * Returns: + * The target scatterlist pointer and the target page position. + * + * Recommended to use wrapper macro: i915_gem_object_page_iter_get_sg() + */ struct scatterlist * -__i915_gem_object_get_sg(struct drm_i915_gem_object *obj, - struct i915_gem_object_page_iter *iter, - unsigned int n, - unsigned int *offset, bool dma); +__i915_gem_object_page_iter_get_sg(struct drm_i915_gem_object *obj, + struct i915_gem_object_page_iter *iter, + pgoff_t n, + unsigned int *offset); +/** + * i915_gem_object_page_iter_get_sg - wrapper macro for + * __i915_gem_object_page_iter_get_sg() + * @obj: i915 GEM buffer object + * @it: i915 GEM buffer object page iterator + * @n: page offset + * @offset: searched physical offset, + * it will be used for returning physical page offset value + * + * Context: Takes and releases the mutex lock of the i915_gem_object_page_iter. + * Takes and releases the RCU lock to search the radix_tree of + * i915_gem_object_page_iter. + * + * Returns: + * The target scatterlist pointer and the target page position. + * + * In order to avoid the truncation of the input parameter, it checks the page + * offset n's type from the input parameter before calling + * __i915_gem_object_page_iter_get_sg(). + */ +#define i915_gem_object_page_iter_get_sg(obj, it, n, offset) ({ \ + assert_typable(pgoff_t, n); \ + __i915_gem_object_page_iter_get_sg(obj, it, n, offset); \ +}) + +/** + * __i915_gem_object_get_sg - helper to find the target scatterlist + * pointer and the target page position using pgoff_t n input argument and + * drm_i915_gem_object. It uses an internal shmem scatterlist lookup function. + * @obj: i915 GEM buffer object + * @n: page offset + * @offset: searched physical offset, + * it will be used for returning physical page offset value + * + * It uses drm_i915_gem_object's internal shmem scatterlist lookup function as + * i915_gem_object_page_iter and calls __i915_gem_object_page_iter_get_sg(). + * + * Returns: + * The target scatterlist pointer and the target page position. + * + * Recommended to use wrapper macro: i915_gem_object_get_sg() + * See also __i915_gem_object_page_iter_get_sg() + */ static inline struct scatterlist * -i915_gem_object_get_sg(struct drm_i915_gem_object *obj, - unsigned int n, - unsigned int *offset) +__i915_gem_object_get_sg(struct drm_i915_gem_object *obj, pgoff_t n, + unsigned int *offset) { - return __i915_gem_object_get_sg(obj, &obj->mm.get_page, n, offset, false); + return __i915_gem_object_page_iter_get_sg(obj, &obj->mm.get_page, n, offset); } +/** + * i915_gem_object_get_sg - wrapper macro for __i915_gem_object_get_sg() + * @obj: i915 GEM buffer object + * @n: page offset + * @offset: searched physical offset, + * it will be used for returning physical page offset value + * + * Returns: + * The target scatterlist pointer and the target page position. + * + * In order to avoid the truncation of the input parameter, it checks the page + * offset n's type from the input parameter before calling + * __i915_gem_object_get_sg(). + * See also __i915_gem_object_page_iter_get_sg() + */ +#define i915_gem_object_get_sg(obj, n, offset) ({ \ + assert_typable(pgoff_t, n); \ + __i915_gem_object_get_sg(obj, n, offset); \ +}) + +/** + * __i915_gem_object_get_sg_dma - helper to find the target scatterlist + * pointer and the target page position using pgoff_t n input argument and + * drm_i915_gem_object. It uses an internal DMA mapped scatterlist lookup function + * @obj: i915 GEM buffer object + * @n: page offset + * @offset: searched physical offset, + * it will be used for returning physical page offset value + * + * It uses drm_i915_gem_object's internal DMA mapped scatterlist lookup function + * as i915_gem_object_page_iter and calls __i915_gem_object_page_iter_get_sg(). + * + * Returns: + * The target scatterlist pointer and the target page position. + * + * Recommended to use wrapper macro: i915_gem_object_get_sg_dma() + * See also __i915_gem_object_page_iter_get_sg() + */ static inline struct scatterlist * -i915_gem_object_get_sg_dma(struct drm_i915_gem_object *obj, - unsigned int n, - unsigned int *offset) +__i915_gem_object_get_sg_dma(struct drm_i915_gem_object *obj, pgoff_t n, + unsigned int *offset) { - return __i915_gem_object_get_sg(obj, &obj->mm.get_dma_page, n, offset, true); + return __i915_gem_object_page_iter_get_sg(obj, &obj->mm.get_dma_page, n, offset); } +/** + * i915_gem_object_get_sg_dma - wrapper macro for __i915_gem_object_get_sg_dma() + * @obj: i915 GEM buffer object + * @n: page offset + * @offset: searched physical offset, + * it will be used for returning physical page offset value + * + * Returns: + * The target scatterlist pointer and the target page position. + * + * In order to avoid the truncation of the input parameter, it checks the page + * offset n's type from the input parameter before calling + * __i915_gem_object_get_sg_dma(). + * See also __i915_gem_object_page_iter_get_sg() + */ +#define i915_gem_object_get_sg_dma(obj, n, offset) ({ \ + assert_typable(pgoff_t, n); \ + __i915_gem_object_get_sg_dma(obj, n, offset); \ +}) + +/** + * __i915_gem_object_get_page - helper to find the target page with a page offset + * @obj: i915 GEM buffer object + * @n: page offset + * + * It uses drm_i915_gem_object's internal shmem scatterlist lookup function as + * i915_gem_object_page_iter and calls __i915_gem_object_page_iter_get_sg() + * internally. + * + * Returns: + * The target page pointer. + * + * Recommended to use wrapper macro: i915_gem_object_get_page() + * See also __i915_gem_object_page_iter_get_sg() + */ struct page * -i915_gem_object_get_page(struct drm_i915_gem_object *obj, - unsigned int n); +__i915_gem_object_get_page(struct drm_i915_gem_object *obj, pgoff_t n); +/** + * i915_gem_object_get_page - wrapper macro for __i915_gem_object_get_page + * @obj: i915 GEM buffer object + * @n: page offset + * + * Returns: + * The target page pointer. + * + * In order to avoid the truncation of the input parameter, it checks the page + * offset n's type from the input parameter before calling + * __i915_gem_object_get_page(). + * See also __i915_gem_object_page_iter_get_sg() + */ +#define i915_gem_object_get_page(obj, n) ({ \ + assert_typable(pgoff_t, n); \ + __i915_gem_object_get_page(obj, n); \ +}) + +/** + * __i915_gem_object_get_dirty_page - helper to find the target page with a page + * offset + * @obj: i915 GEM buffer object + * @n: page offset + * + * It works like i915_gem_object_get_page(), but it marks the returned page dirty. + * + * Returns: + * The target page pointer. + * + * Recommended to use wrapper macro: i915_gem_object_get_dirty_page() + * See also __i915_gem_object_page_iter_get_sg() and __i915_gem_object_get_page() + */ struct page * -i915_gem_object_get_dirty_page(struct drm_i915_gem_object *obj, - unsigned int n); +__i915_gem_object_get_dirty_page(struct drm_i915_gem_object *obj, pgoff_t n); + +/** + * i915_gem_object_get_dirty_page - wrapper macro for __i915_gem_object_get_dirty_page + * @obj: i915 GEM buffer object + * @n: page offset + * + * Returns: + * The target page pointer. + * + * In order to avoid the truncation of the input parameter, it checks the page + * offset n's type from the input parameter before calling + * __i915_gem_object_get_dirty_page(). + * See also __i915_gem_object_page_iter_get_sg() and __i915_gem_object_get_page() + */ +#define i915_gem_object_get_dirty_page(obj, n) ({ \ + assert_typable(pgoff_t, n); \ + __i915_gem_object_get_dirty_page(obj, n); \ +}) +/** + * __i915_gem_object_get_dma_address_len - helper to get bus addresses of + * targeted DMA mapped scatterlist from i915 GEM buffer object and it's length + * @obj: i915 GEM buffer object + * @n: page offset + * @len: DMA mapped scatterlist's DMA bus addresses length to return + * + * Returns: + * Bus addresses of targeted DMA mapped scatterlist + * + * Recommended to use wrapper macro: i915_gem_object_get_dma_address_len() + * See also __i915_gem_object_page_iter_get_sg() and __i915_gem_object_get_sg_dma() + */ dma_addr_t -i915_gem_object_get_dma_address_len(struct drm_i915_gem_object *obj, - unsigned long n, - unsigned int *len); +__i915_gem_object_get_dma_address_len(struct drm_i915_gem_object *obj, pgoff_t n, + unsigned int *len); +/** + * i915_gem_object_get_dma_address_len - wrapper macro for + * __i915_gem_object_get_dma_address_len + * @obj: i915 GEM buffer object + * @n: page offset + * @len: DMA mapped scatterlist's DMA bus addresses length to return + * + * Returns: + * Bus addresses of targeted DMA mapped scatterlist + * + * In order to avoid the truncation of the input parameter, it checks the page + * offset n's type from the input parameter before calling + * __i915_gem_object_get_dma_address_len(). + * See also __i915_gem_object_page_iter_get_sg() and + * __i915_gem_object_get_dma_address_len() + */ +#define i915_gem_object_get_dma_address_len(obj, n, len) ({ \ + assert_typable(pgoff_t, n); \ + __i915_gem_object_get_dma_address_len(obj, n, len); \ +}) + +/** + * __i915_gem_object_get_dma_address - helper to get bus addresses of + * targeted DMA mapped scatterlist from i915 GEM buffer object + * @obj: i915 GEM buffer object + * @n: page offset + * + * Returns: + * Bus addresses of targeted DMA mapped scatterlis + * + * Recommended to use wrapper macro: i915_gem_object_get_dma_address() + * See also __i915_gem_object_page_iter_get_sg() and __i915_gem_object_get_sg_dma() + */ dma_addr_t -i915_gem_object_get_dma_address(struct drm_i915_gem_object *obj, - unsigned long n); +__i915_gem_object_get_dma_address(struct drm_i915_gem_object *obj, pgoff_t n); + +/** + * i915_gem_object_get_dma_address - wrapper macro for + * __i915_gem_object_get_dma_address + * @obj: i915 GEM buffer object + * @n: page offset + * + * Returns: + * Bus addresses of targeted DMA mapped scatterlist + * + * In order to avoid the truncation of the input parameter, it checks the page + * offset n's type from the input parameter before calling + * __i915_gem_object_get_dma_address(). + * See also __i915_gem_object_page_iter_get_sg() and + * __i915_gem_object_get_dma_address() + */ +#define i915_gem_object_get_dma_address(obj, n) ({ \ + assert_typable(pgoff_t, n); \ + __i915_gem_object_get_dma_address(obj, n); \ +}) void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj, struct sg_table *pages, diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c index 4df50b049cea..015b6cd071e4 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c @@ -510,14 +510,16 @@ void __i915_gem_object_release_map(struct drm_i915_gem_object *obj) } struct scatterlist * -__i915_gem_object_get_sg(struct drm_i915_gem_object *obj, - struct i915_gem_object_page_iter *iter, - unsigned int n, - unsigned int *offset, - bool dma) +__i915_gem_object_page_iter_get_sg(struct drm_i915_gem_object *obj, + struct i915_gem_object_page_iter *iter, + pgoff_t n, + unsigned int *offset) + { - struct scatterlist *sg; + const bool dma = iter == &obj->mm.get_dma_page || + iter == &obj->ttm.get_io_page; unsigned int idx, count; + struct scatterlist *sg; might_sleep(); GEM_BUG_ON(n >= obj->base.size >> PAGE_SHIFT); @@ -625,7 +627,7 @@ __i915_gem_object_get_sg(struct drm_i915_gem_object *obj, } struct page * -i915_gem_object_get_page(struct drm_i915_gem_object *obj, unsigned int n) +__i915_gem_object_get_page(struct drm_i915_gem_object *obj, pgoff_t n) { struct scatterlist *sg; unsigned int offset; @@ -638,8 +640,7 @@ i915_gem_object_get_page(struct drm_i915_gem_object *obj, unsigned int n) /* Like i915_gem_object_get_page(), but mark the returned page dirty */ struct page * -i915_gem_object_get_dirty_page(struct drm_i915_gem_object *obj, - unsigned int n) +__i915_gem_object_get_dirty_page(struct drm_i915_gem_object *obj, pgoff_t n) { struct page *page; @@ -651,9 +652,8 @@ i915_gem_object_get_dirty_page(struct drm_i915_gem_object *obj, } dma_addr_t -i915_gem_object_get_dma_address_len(struct drm_i915_gem_object *obj, - unsigned long n, - unsigned int *len) +__i915_gem_object_get_dma_address_len(struct drm_i915_gem_object *obj, + pgoff_t n, unsigned int *len) { struct scatterlist *sg; unsigned int offset; @@ -667,8 +667,7 @@ i915_gem_object_get_dma_address_len(struct drm_i915_gem_object *obj, } dma_addr_t -i915_gem_object_get_dma_address(struct drm_i915_gem_object *obj, - unsigned long n) +__i915_gem_object_get_dma_address(struct drm_i915_gem_object *obj, pgoff_t n) { return i915_gem_object_get_dma_address_len(obj, n, NULL); } diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index f64a3deb12fc..b6c813d69ffa 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -685,7 +685,7 @@ static unsigned long i915_ttm_io_mem_pfn(struct ttm_buffer_object *bo, GEM_WARN_ON(bo->ttm); base = obj->mm.region->iomap.base - obj->mm.region->region.start; - sg = __i915_gem_object_get_sg(obj, &obj->ttm.get_io_page, page_offset, &ofs, true); + sg = i915_gem_object_page_iter_get_sg(obj, &obj->ttm.get_io_page, page_offset, &ofs); return ((base + sg_dma_address(sg)) >> PAGE_SHIFT) + ofs; } diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c index c6ad67b90e8a..a18a890e681f 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c @@ -455,7 +455,8 @@ static int gpu_fill(struct intel_context *ce, static int cpu_fill(struct drm_i915_gem_object *obj, u32 value) { const bool has_llc = HAS_LLC(to_i915(obj->base.dev)); - unsigned int n, m, need_flush; + unsigned int need_flush; + unsigned long n, m; int err; i915_gem_object_lock(obj, NULL); @@ -485,7 +486,8 @@ static int cpu_fill(struct drm_i915_gem_object *obj, u32 value) static noinline int cpu_check(struct drm_i915_gem_object *obj, unsigned int idx, unsigned int max) { - unsigned int n, m, needs_flush; + unsigned int needs_flush; + unsigned long n; int err; i915_gem_object_lock(obj, NULL); @@ -494,7 +496,7 @@ static noinline int cpu_check(struct drm_i915_gem_object *obj, goto out_unlock; for (n = 0; n < real_page_count(obj); n++) { - u32 *map; + u32 *map, m; map = kmap_atomic(i915_gem_object_get_page(obj, n)); if (needs_flush & CLFLUSH_BEFORE) @@ -502,7 +504,7 @@ static noinline int cpu_check(struct drm_i915_gem_object *obj, for (m = 0; m < max; m++) { if (map[m] != m) { - pr_err("%pS: Invalid value at object %d page %d/%ld, offset %d/%d: found %x expected %x\n", + pr_err("%pS: Invalid value at object %d page %ld/%ld, offset %d/%d: found %x expected %x\n", __builtin_return_address(0), idx, n, real_page_count(obj), m, max, map[m], m); @@ -513,7 +515,7 @@ static noinline int cpu_check(struct drm_i915_gem_object *obj, for (; m < DW_PER_PAGE; m++) { if (map[m] != STACK_MAGIC) { - pr_err("%pS: Invalid value at object %d page %d, offset %d: found %x expected %x (uninitialised)\n", + pr_err("%pS: Invalid value at object %d page %ld, offset %d: found %x expected %x (uninitialised)\n", __builtin_return_address(0), idx, n, m, map[m], STACK_MAGIC); err = -EINVAL; diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c index b73c91aa5450..4ea6e48d8689 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c @@ -95,11 +95,11 @@ static int check_partial_mapping(struct drm_i915_gem_object *obj, struct drm_i915_private *i915 = to_i915(obj->base.dev); struct i915_gtt_view view; struct i915_vma *vma; + unsigned long offset; unsigned long page; u32 __iomem *io; struct page *p; unsigned int n; - u64 offset; u32 *cpu; int err; @@ -156,7 +156,7 @@ static int check_partial_mapping(struct drm_i915_gem_object *obj, cpu = kmap(p) + offset_in_page(offset); drm_clflush_virt_range(cpu, sizeof(*cpu)); if (*cpu != (u32)page) { - pr_err("Partial view for %lu [%u] (offset=%llu, size=%u [%llu, row size %u], fence=%d, tiling=%d, stride=%d) misalignment, expected write to page (%llu + %u [0x%llx]) of 0x%x, found 0x%x\n", + pr_err("Partial view for %lu [%u] (offset=%llu, size=%u [%llu, row size %u], fence=%d, tiling=%d, stride=%d) misalignment, expected write to page (%lu + %u [0x%lx]) of 0x%x, found 0x%x\n", page, n, view.partial.offset, view.partial.size, @@ -212,10 +212,10 @@ static int check_partial_mappings(struct drm_i915_gem_object *obj, for_each_prime_number_from(page, 1, npages) { struct i915_gtt_view view = compute_partial_view(obj, page, MIN_CHUNK_PAGES); + unsigned long offset; u32 __iomem *io; struct page *p; unsigned int n; - u64 offset; u32 *cpu; GEM_BUG_ON(view.partial.size > nreal); @@ -252,7 +252,7 @@ static int check_partial_mappings(struct drm_i915_gem_object *obj, cpu = kmap(p) + offset_in_page(offset); drm_clflush_virt_range(cpu, sizeof(*cpu)); if (*cpu != (u32)page) { - pr_err("Partial view for %lu [%u] (offset=%llu, size=%u [%llu, row size %u], fence=%d, tiling=%d, stride=%d) misalignment, expected write to page (%llu + %u [0x%llx]) of 0x%x, found 0x%x\n", + pr_err("Partial view for %lu [%u] (offset=%llu, size=%u [%llu, row size %u], fence=%d, tiling=%d, stride=%d) misalignment, expected write to page (%lu + %u [0x%lx]) of 0x%x, found 0x%x\n", page, n, view.partial.offset, view.partial.size, diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c index bdf5bb40ccf1..19e374f68ff7 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c @@ -33,10 +33,10 @@ static int igt_gem_object(void *arg) static int igt_gem_huge(void *arg) { - const unsigned int nreal = 509; /* just to be awkward */ + const unsigned long nreal = 509; /* just to be awkward */ struct drm_i915_private *i915 = arg; struct drm_i915_gem_object *obj; - unsigned int n; + unsigned long n; int err; /* Basic sanitycheck of our huge fake object allocation */ @@ -49,7 +49,7 @@ static int igt_gem_huge(void *arg) err = i915_gem_object_pin_pages_unlocked(obj); if (err) { - pr_err("Failed to allocate %u pages (%lu total), err=%d\n", + pr_err("Failed to allocate %lu pages (%lu total), err=%d\n", nreal, obj->base.size / PAGE_SIZE, err); goto out; } @@ -57,7 +57,7 @@ static int igt_gem_huge(void *arg) for (n = 0; n < obj->base.size / PAGE_SIZE; n++) { if (i915_gem_object_get_page(obj, n) != i915_gem_object_get_page(obj, n % nreal)) { - pr_err("Page lookup mismatch at index %u [%u]\n", + pr_err("Page lookup mismatch at index %lu [%lu]\n", n, n % nreal); err = -EINVAL; goto out_unpin; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index bae857d5221d..d82fc4abc348 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -229,8 +229,9 @@ i915_gem_shmem_pread(struct drm_i915_gem_object *obj, struct drm_i915_gem_pread *args) { unsigned int needs_clflush; - unsigned int idx, offset; char __user *user_data; + unsigned long offset; + pgoff_t idx; u64 remain; int ret; @@ -383,13 +384,17 @@ i915_gem_gtt_pread(struct drm_i915_gem_object *obj, { struct drm_i915_private *i915 = to_i915(obj->base.dev); struct i915_ggtt *ggtt = to_gt(i915)->ggtt; + unsigned long remain, offset; intel_wakeref_t wakeref; struct drm_mm_node node; void __user *user_data; struct i915_vma *vma; - u64 remain, offset; int ret = 0; + if (overflows_type(args->size, remain) || + overflows_type(args->offset, offset)) + return -EINVAL; + wakeref = intel_runtime_pm_get(&i915->runtime_pm); vma = i915_gem_gtt_prepare(obj, &node, false); @@ -540,13 +545,17 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj, struct drm_i915_private *i915 = to_i915(obj->base.dev); struct i915_ggtt *ggtt = to_gt(i915)->ggtt; struct intel_runtime_pm *rpm = &i915->runtime_pm; + unsigned long remain, offset; intel_wakeref_t wakeref; struct drm_mm_node node; struct i915_vma *vma; - u64 remain, offset; void __user *user_data; int ret = 0; + if (overflows_type(args->size, remain) || + overflows_type(args->offset, offset)) + return -EINVAL; + if (i915_gem_object_has_struct_page(obj)) { /* * Avoid waking the device up if we can fallback, as @@ -654,8 +663,9 @@ i915_gem_shmem_pwrite(struct drm_i915_gem_object *obj, { unsigned int partial_cacheline_write; unsigned int needs_clflush; - unsigned int offset, idx; void __user *user_data; + unsigned long offset; + pgoff_t idx; u64 remain; int ret; diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h index efd3d69b78f7..d2144986cbf1 100644 --- a/drivers/gpu/drm/i915/i915_utils.h +++ b/drivers/gpu/drm/i915/i915_utils.h @@ -33,6 +33,7 @@ #include #include #include +#include #ifdef CONFIG_X86 #include diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c index f17c09ead7d7..bf3a30cd02bf 100644 --- a/drivers/gpu/drm/i915/i915_vma.c +++ b/drivers/gpu/drm/i915/i915_vma.c @@ -909,7 +909,7 @@ rotate_pages(struct drm_i915_gem_object *obj, unsigned int offset, struct sg_table *st, struct scatterlist *sg) { unsigned int column, row; - unsigned int src_idx; + pgoff_t src_idx; for (column = 0; column < width; column++) { unsigned int left; @@ -1015,7 +1015,7 @@ add_padding_pages(unsigned int count, static struct scatterlist * remap_tiled_color_plane_pages(struct drm_i915_gem_object *obj, - unsigned int offset, unsigned int alignment_pad, + unsigned long offset, unsigned int alignment_pad, unsigned int width, unsigned int height, unsigned int src_stride, unsigned int dst_stride, struct sg_table *st, struct scatterlist *sg, @@ -1074,7 +1074,7 @@ remap_tiled_color_plane_pages(struct drm_i915_gem_object *obj, static struct scatterlist * remap_contiguous_pages(struct drm_i915_gem_object *obj, - unsigned int obj_offset, + pgoff_t obj_offset, unsigned int count, struct sg_table *st, struct scatterlist *sg) { @@ -1107,7 +1107,7 @@ remap_contiguous_pages(struct drm_i915_gem_object *obj, static struct scatterlist * remap_linear_color_plane_pages(struct drm_i915_gem_object *obj, - unsigned int obj_offset, unsigned int alignment_pad, + pgoff_t obj_offset, unsigned int alignment_pad, unsigned int size, struct sg_table *st, struct scatterlist *sg, unsigned int *gtt_offset) From patchwork Fri Sep 9 10:59:09 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Gwan-gyeong Mun X-Patchwork-Id: 12971535 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 10A75ECAAD3 for ; Fri, 9 Sep 2022 11:00:51 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id BFCDE10EC82; Fri, 9 Sep 2022 11:00:05 +0000 (UTC) Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id A3BC510EC6E; Fri, 9 Sep 2022 10:59:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1662721196; x=1694257196; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=olw9GjlAEK6rdjOB4E7bzLuw16Gmb03f4cqTnjjsmHI=; b=ZsZjjDJ17G1ZeN+Ga1Q5D+YDOjZ/qPtiOC+FPehaY9dz8L3jFmEjoAp+ T2BGFoaHWXqjetJ/Fdqwuy9nmD6jZRChWo6wtk01/iBLF9cYMqGMR3WrN JswvbN+X4iRK95kpXdgXyxna9mHtMgwTamtOYh5Y6BKAPgzg3tQCkkOM6 7hABS+OvPLl9ZS8dpFdIDVKnFXWxeLpj5+StrCnwe81ypmLdcbsCrc7oD Na7Hvt24kpg1ZtgmskWaNDo/t8+7iF2bXdiHAI4Na56YLX8uxDNLMm9ll wFOa6BiiSTxxJzu7eR3qjRjBy05B7JNGY6DWg0flp/MwZKVYv52tlvaXM g==; X-IronPort-AV: E=McAfee;i="6500,9779,10464"; a="359174685" X-IronPort-AV: E=Sophos;i="5.93,303,1654585200"; d="scan'208";a="359174685" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Sep 2022 03:59:56 -0700 X-IronPort-AV: E=Sophos;i="5.93,303,1654585200"; d="scan'208";a="704370371" Received: from clbarnes-mobl.amr.corp.intel.com (HELO paris.amr.corp.intel.com) ([10.254.42.92]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Sep 2022 03:59:52 -0700 From: Gwan-gyeong Mun To: intel-gfx@lists.freedesktop.org Date: Fri, 9 Sep 2022 19:59:09 +0900 Message-Id: <20220909105913.752049-6-gwan-gyeong.mun@intel.com> X-Mailer: git-send-email 2.37.1 In-Reply-To: <20220909105913.752049-1-gwan-gyeong.mun@intel.com> References: <20220909105913.752049-1-gwan-gyeong.mun@intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH v10 5/9] drm/i915: Check for integer truncation on scatterlist creation X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: thomas.hellstrom@linux.intel.com, keescook@chromium.org, jani.nikula@intel.com, ndesaulniers@google.com, dlatypov@google.com, linux@rasmusvillemoes.dk, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, chris@chris-wilson.co.uk, andrzej.hajda@intel.com, matthew.auld@intel.com, daniel@ffwll.ch, airlied@redhat.com, mchehab@kernel.org, vitor@massaru.org, nirmoy.das@intel.com Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" From: Chris Wilson There is an impedance mismatch between the scatterlist API using unsigned int and our memory/page accounting in unsigned long. That is we may try to create a scatterlist for a large object that overflows returning a small table into which we try to fit very many pages. As the object size is under control of userspace, we have to be prudent and catch the conversion errors. To catch the implicit truncation as we switch from unsigned long into the scatterlist's unsigned int, we use overflows_type check and report E2BIG prior to the operation. This is already used in our create ioctls to indicate if the uABI request is simply too large for the backing store. Failing that type check, we have a second check at sg_alloc_table time to make sure the values we are passing into the scatterlist API are not truncated. It uses pgoff_t for locals that are dealing with page indices, in this case, the page count is the limit of the page index. And it uses safe_conversion() macro which performs a type conversion (cast) of an integer value into a new variable, checking that the destination is large enough to hold the source value. v2: Move added i915_utils's macro into drm_util header (Jani N) v5: Fix macros to be enclosed in parentheses for complex values Fix too long line warning v8: Replace safe_conversion() with check_assign() (Kees) Signed-off-by: Chris Wilson Signed-off-by: Gwan-gyeong Mun Cc: Tvrtko Ursulin Cc: Brian Welty Cc: Matthew Auld Cc: Thomas Hellström Reviewed-by: Nirmoy Das Reviewed-by: Mauro Carvalho Chehab Reviewed-by: Andrzej Hajda --- drivers/gpu/drm/i915/gem/i915_gem_internal.c | 6 ++++-- drivers/gpu/drm/i915/gem/i915_gem_object.h | 3 --- drivers/gpu/drm/i915/gem/i915_gem_phys.c | 4 ++++ drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 5 ++++- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 4 ++++ drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 5 ++++- drivers/gpu/drm/i915/gvt/dmabuf.c | 9 +++++---- drivers/gpu/drm/i915/i915_scatterlist.h | 11 +++++++++++ 8 files changed, 36 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_internal.c b/drivers/gpu/drm/i915/gem/i915_gem_internal.c index c698f95af15f..53fa27e1c950 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_internal.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_internal.c @@ -37,10 +37,13 @@ static int i915_gem_object_get_pages_internal(struct drm_i915_gem_object *obj) struct sg_table *st; struct scatterlist *sg; unsigned int sg_page_sizes; - unsigned int npages; + pgoff_t npages; /* restricted by sg_alloc_table */ int max_order; gfp_t gfp; + if (check_assign(obj->base.size >> PAGE_SHIFT, &npages)) + return -E2BIG; + max_order = MAX_ORDER; #ifdef CONFIG_SWIOTLB if (is_swiotlb_active(obj->base.dev->dev)) { @@ -67,7 +70,6 @@ static int i915_gem_object_get_pages_internal(struct drm_i915_gem_object *obj) if (!st) return -ENOMEM; - npages = obj->base.size / PAGE_SIZE; if (sg_alloc_table(st, npages, GFP_KERNEL)) { kfree(st); return -ENOMEM; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h index 8dd2b84468c8..a64fe73c05b5 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h @@ -26,9 +26,6 @@ enum intel_region_id; * this and catch if we ever need to fix it. In the meantime, if you do * spot such a local variable, please consider fixing! * - * Aside from our own locals (for which we have no excuse!): - * - sg_table embeds unsigned int for nents - * * We can check for invalidly typed locals with typecheck(), see for example * i915_gem_object_get_sg(). */ diff --git a/drivers/gpu/drm/i915/gem/i915_gem_phys.c b/drivers/gpu/drm/i915/gem/i915_gem_phys.c index 0d0e46dae559..88ba7266a3a5 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_phys.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_phys.c @@ -28,6 +28,10 @@ static int i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj) void *dst; int i; + /* Contiguous chunk, with a single scatterlist element */ + if (overflows_type(obj->base.size, sg->length)) + return -E2BIG; + if (GEM_WARN_ON(i915_gem_object_needs_bit17_swizzle(obj))) return -EINVAL; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c index f42ca1179f37..339b0a9cf2d0 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c @@ -193,13 +193,16 @@ static int shmem_get_pages(struct drm_i915_gem_object *obj) struct drm_i915_private *i915 = to_i915(obj->base.dev); struct intel_memory_region *mem = obj->mm.region; struct address_space *mapping = obj->base.filp->f_mapping; - const unsigned long page_count = obj->base.size / PAGE_SIZE; unsigned int max_segment = i915_sg_segment_size(); struct sg_table *st; struct sgt_iter sgt_iter; + pgoff_t page_count; struct page *page; int ret; + if (check_assign(obj->base.size >> PAGE_SHIFT, &page_count)) + return -E2BIG; + /* * Assert that the object is not currently in any GPU domain. As it * wasn't in the GTT, there shouldn't be any way it could have been in diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index b6c813d69ffa..06a2e80a5702 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -783,6 +783,10 @@ static int i915_ttm_get_pages(struct drm_i915_gem_object *obj) { struct ttm_place requested, busy[I915_TTM_MAX_PLACEMENTS]; struct ttm_placement placement; + pgoff_t num_pages; + + if (check_assign(obj->base.size >> PAGE_SHIFT, &num_pages)) + return -E2BIG; GEM_BUG_ON(obj->mm.n_placements > I915_TTM_MAX_PLACEMENTS); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c index 8423df021b71..48237e443863 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c @@ -128,13 +128,16 @@ static void i915_gem_object_userptr_drop_ref(struct drm_i915_gem_object *obj) static int i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj) { - const unsigned long num_pages = obj->base.size >> PAGE_SHIFT; unsigned int max_segment = i915_sg_segment_size(); struct sg_table *st; unsigned int sg_page_sizes; struct page **pvec; + pgoff_t num_pages; /* limited by sg_alloc_table_from_pages_segment */ int ret; + if (check_assign(obj->base.size >> PAGE_SHIFT, &num_pages)) + return -E2BIG; + st = kmalloc(sizeof(*st), GFP_KERNEL); if (!st) return -ENOMEM; diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c b/drivers/gpu/drm/i915/gvt/dmabuf.c index 01e54b45c5c1..bc6e823584ad 100644 --- a/drivers/gpu/drm/i915/gvt/dmabuf.c +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c @@ -42,8 +42,7 @@ #define GEN8_DECODE_PTE(pte) (pte & GENMASK_ULL(63, 12)) -static int vgpu_gem_get_pages( - struct drm_i915_gem_object *obj) +static int vgpu_gem_get_pages(struct drm_i915_gem_object *obj) { struct drm_i915_private *dev_priv = to_i915(obj->base.dev); struct intel_vgpu *vgpu; @@ -52,7 +51,10 @@ static int vgpu_gem_get_pages( int i, j, ret; gen8_pte_t __iomem *gtt_entries; struct intel_vgpu_fb_info *fb_info; - u32 page_num; + pgoff_t page_num; + + if (check_assign(obj->base.size >> PAGE_SHIFT, &page_num)) + return -E2BIG; fb_info = (struct intel_vgpu_fb_info *)obj->gvt_info; if (drm_WARN_ON(&dev_priv->drm, !fb_info)) @@ -66,7 +68,6 @@ static int vgpu_gem_get_pages( if (unlikely(!st)) return -ENOMEM; - page_num = obj->base.size >> PAGE_SHIFT; ret = sg_alloc_table(st, page_num, GFP_KERNEL); if (ret) { kfree(st); diff --git a/drivers/gpu/drm/i915/i915_scatterlist.h b/drivers/gpu/drm/i915/i915_scatterlist.h index 9ddb3e743a3e..1d1802beb42b 100644 --- a/drivers/gpu/drm/i915/i915_scatterlist.h +++ b/drivers/gpu/drm/i915/i915_scatterlist.h @@ -220,4 +220,15 @@ struct i915_refct_sgt *i915_rsgt_from_buddy_resource(struct ttm_resource *res, u64 region_start, u32 page_alignment); +/* Wrap scatterlist.h to sanity check for integer truncation */ +typedef unsigned int __sg_size_t; /* see linux/scatterlist.h */ +#define sg_alloc_table(sgt, nents, gfp) \ + overflows_type(nents, __sg_size_t) ? -E2BIG \ + : ((sg_alloc_table)(sgt, (__sg_size_t)(nents), gfp)) + +#define sg_alloc_table_from_pages_segment(sgt, pages, npages, offset, size, max_segment, gfp) \ + overflows_type(npages, __sg_size_t) ? -E2BIG \ + : ((sg_alloc_table_from_pages_segment)(sgt, pages, (__sg_size_t)(npages), offset, \ + size, max_segment, gfp)) + #endif From patchwork Fri Sep 9 10:59:10 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Gwan-gyeong Mun X-Patchwork-Id: 12971538 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 9AE17ECAAA1 for ; Fri, 9 Sep 2022 11:01:06 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 7038010EC85; Fri, 9 Sep 2022 11:00:25 +0000 (UTC) Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2AF1310EC77; Fri, 9 Sep 2022 11:00:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1662721200; x=1694257200; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=nn1a9tFrsZga0A6pfl0862R63n1W3dcE7KCEwy2yz0Y=; b=TJ6g2816M1Fsawt8Aw7pJBzfaB2mdcCGaRoGFSGj0YSdNcCTTcuD4D6I tij3QmZNDQTfmncyHA66HIOVg9s1VWvOewIdjuQF0AzNltHL5r9KR0poU eD6C1zTd3PTgZFRBHmUgQ5WVe/7eZN2Pvlr22hnkd6w60HtqqpXsccdD0 3TxLv9/M8UnXxo1GE9FCcvEEWo4ATtzHaM7GhjgzBTSFZN3itBHdSjTLl Iulf0x0+nHf+aUKlFcAK9zKI9MNp+Vf7wCQVgBLtz2SZqxG79rKG9bY6G uNotwNSOdzOZiOkx12XPpCtADv5HR+CQoPg8dOrEbYtRFV1KAvVmClh67 Q==; X-IronPort-AV: E=McAfee;i="6500,9779,10464"; a="359174691" X-IronPort-AV: E=Sophos;i="5.93,303,1654585200"; d="scan'208";a="359174691" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Sep 2022 03:59:59 -0700 X-IronPort-AV: E=Sophos;i="5.93,303,1654585200"; d="scan'208";a="704370380" Received: from clbarnes-mobl.amr.corp.intel.com (HELO paris.amr.corp.intel.com) ([10.254.42.92]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Sep 2022 03:59:56 -0700 From: Gwan-gyeong Mun To: intel-gfx@lists.freedesktop.org Date: Fri, 9 Sep 2022 19:59:10 +0900 Message-Id: <20220909105913.752049-7-gwan-gyeong.mun@intel.com> X-Mailer: git-send-email 2.37.1 In-Reply-To: <20220909105913.752049-1-gwan-gyeong.mun@intel.com> References: <20220909105913.752049-1-gwan-gyeong.mun@intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH v10 6/9] drm/i915: Check for integer truncation on the configuration of ttm place X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: thomas.hellstrom@linux.intel.com, keescook@chromium.org, jani.nikula@intel.com, ndesaulniers@google.com, dlatypov@google.com, linux@rasmusvillemoes.dk, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, chris@chris-wilson.co.uk, andrzej.hajda@intel.com, matthew.auld@intel.com, daniel@ffwll.ch, airlied@redhat.com, mchehab@kernel.org, vitor@massaru.org, nirmoy.das@intel.com Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" 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 Cc: Chris Wilson Cc: Matthew Auld Cc: Thomas Hellström Cc: Jani Nikula Reviewed-by: Nirmoy Das (v2) Reviewed-by: Mauro Carvalho Chehab (v3) Reported-by: kernel test robot Reviewed-by: Andrzej Hajda (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 06a2e80a5702..6956d273fa5f 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) From patchwork Fri Sep 9 10:59:11 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Gwan-gyeong Mun X-Patchwork-Id: 12971536 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 850B8C6FA82 for ; Fri, 9 Sep 2022 11:00:52 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id DE05810EC72; Fri, 9 Sep 2022 11:00:11 +0000 (UTC) Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id B124710EC77; Fri, 9 Sep 2022 11:00:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1662721203; x=1694257203; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=fusilmdSQD+FIBKYeUukiHJrxKRQ3B5pPSTTQeO8rTk=; b=IY4ByzcKZWkyGAT5DBxY8zhM1XHXGdCwRIhTUgBo5KFW4yns+wc2TnjB tgRZakZlh31BJ2HopaTvfTtVAlvGXoqrrRjq/1QPPxhH7vWPApzoiIWv/ G8BHlstQNThIlkMCKZRm2S81V3wjv8g0kzfseMhHjCoY/m6plwBADRsur WjfCtHK74IyGaGzvQiP626M1qV6Mqoh9Exzc0KiuRWail39aDcHMqXs3Y jPI47H3xtcfjECb/CduFd7j0pk2RuCxGt92MHCSOuPmOAetGP79ar/bed +c6j7jDZYYSBFueIjM5mUcDqX0xO9DI0ArDH4X53hOz/Ij2KibyEREemL A==; X-IronPort-AV: E=McAfee;i="6500,9779,10464"; a="359174707" X-IronPort-AV: E=Sophos;i="5.93,303,1654585200"; d="scan'208";a="359174707" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Sep 2022 04:00:03 -0700 X-IronPort-AV: E=Sophos;i="5.93,303,1654585200"; d="scan'208";a="704370438" Received: from clbarnes-mobl.amr.corp.intel.com (HELO paris.amr.corp.intel.com) ([10.254.42.92]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Sep 2022 04:00:00 -0700 From: Gwan-gyeong Mun To: intel-gfx@lists.freedesktop.org Date: Fri, 9 Sep 2022 19:59:11 +0900 Message-Id: <20220909105913.752049-8-gwan-gyeong.mun@intel.com> X-Mailer: git-send-email 2.37.1 In-Reply-To: <20220909105913.752049-1-gwan-gyeong.mun@intel.com> References: <20220909105913.752049-1-gwan-gyeong.mun@intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH v10 7/9] drm/i915: Check if the size is too big while creating shmem file X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: thomas.hellstrom@linux.intel.com, keescook@chromium.org, jani.nikula@intel.com, ndesaulniers@google.com, dlatypov@google.com, linux@rasmusvillemoes.dk, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, chris@chris-wilson.co.uk, andrzej.hajda@intel.com, matthew.auld@intel.com, daniel@ffwll.ch, airlied@redhat.com, mchehab@kernel.org, vitor@massaru.org, nirmoy.das@intel.com Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" The __shmem_file_setup() function returns -EINVAL if size is greater than MAX_LFS_FILESIZE. To handle the same error as other code that returns -E2BIG when the size is too large, it add a code that returns -E2BIG when the size is larger than the size that can be handled. v4: If BITS_PER_LONG is 32, size > MAX_LFS_FILESIZE is always false, so it checks only when BITS_PER_LONG is 64. Signed-off-by: Gwan-gyeong Mun Cc: Chris Wilson Cc: Matthew Auld Cc: Thomas Hellström Reviewed-by: Nirmoy Das Reviewed-by: Mauro Carvalho Chehab Reported-by: kernel test robot Reviewed-by: Andrzej Hajda --- drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c index 339b0a9cf2d0..ca30060e34ab 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c @@ -541,6 +541,20 @@ static int __create_shmem(struct drm_i915_private *i915, drm_gem_private_object_init(&i915->drm, obj, size); + /* XXX: The __shmem_file_setup() function returns -EINVAL if size is + * greater than MAX_LFS_FILESIZE. + * To handle the same error as other code that returns -E2BIG when + * the size is too large, we add a code that returns -E2BIG when the + * size is larger than the size that can be handled. + * If BITS_PER_LONG is 32, size > MAX_LFS_FILESIZE is always false, + * so we only needs to check when BITS_PER_LONG is 64. + * If BITS_PER_LONG is 32, E2BIG checks are processed when + * i915_gem_object_size_2big() is called before init_object() callback + * is called. + */ + if (BITS_PER_LONG == 64 && size > MAX_LFS_FILESIZE) + return -E2BIG; + if (i915->mm.gemfs) filp = shmem_file_setup_with_mnt(i915->mm.gemfs, "i915", size, flags); From patchwork Fri Sep 9 10:59:12 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Gwan-gyeong Mun X-Patchwork-Id: 12971539 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C0B28C6FA82 for ; Fri, 9 Sep 2022 11:01:07 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id AB61F10EC88; Fri, 9 Sep 2022 11:00:26 +0000 (UTC) Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 4589510EC67; Fri, 9 Sep 2022 11:00:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1662721207; x=1694257207; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=rbeLLMikeeyGnzJWYLFEPrxR618A28YgVik17/AHfoY=; b=fadAGEUlRMTDOvHoTe/Gxg7S5kjFjlhOrgzcDYpZhpSH+RYt7qcmNZ7L EuXZm9fVeS40nwufG+Vlv3UTwsyeuv0FntteHcdNgO+P43M25HZ1lgQXo mEJKhOjiONPrR/1Rp9GfrS+A0QAbyZ+U0ibXdI/3IG5xcb1NYM6qO9Y0M msRUZdmjO9PWTKcWhvJvAAe/OUNrh0V8yDDSWOmryj9I/wSOZ/B1OGlbD SrYhAkNry5HDoMnRbnwJNeDZqTdycaRcTcxKKB6M7VP7CoSK4Lb0JdEl9 fmk+h3Ofm+2RpnsukmhIulwTVz+omFXP+cW65ddwbO9wtbtX3mRh4K2o3 Q==; X-IronPort-AV: E=McAfee;i="6500,9779,10464"; a="359174715" X-IronPort-AV: E=Sophos;i="5.93,303,1654585200"; d="scan'208";a="359174715" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Sep 2022 04:00:07 -0700 X-IronPort-AV: E=Sophos;i="5.93,303,1654585200"; d="scan'208";a="704370507" Received: from clbarnes-mobl.amr.corp.intel.com (HELO paris.amr.corp.intel.com) ([10.254.42.92]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Sep 2022 04:00:03 -0700 From: Gwan-gyeong Mun To: intel-gfx@lists.freedesktop.org Date: Fri, 9 Sep 2022 19:59:12 +0900 Message-Id: <20220909105913.752049-9-gwan-gyeong.mun@intel.com> X-Mailer: git-send-email 2.37.1 In-Reply-To: <20220909105913.752049-1-gwan-gyeong.mun@intel.com> References: <20220909105913.752049-1-gwan-gyeong.mun@intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH v10 8/9] drm/i915: Use error code as -E2BIG when the size of gem ttm object is too large X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: thomas.hellstrom@linux.intel.com, keescook@chromium.org, jani.nikula@intel.com, ndesaulniers@google.com, dlatypov@google.com, linux@rasmusvillemoes.dk, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, chris@chris-wilson.co.uk, andrzej.hajda@intel.com, matthew.auld@intel.com, daniel@ffwll.ch, airlied@redhat.com, mchehab@kernel.org, vitor@massaru.org, nirmoy.das@intel.com Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" The ttm_bo_init_reserved() functions returns -ENOSPC if the size is too big to add vma. The direct function that returns -ENOSPC is drm_mm_insert_node_in_range(). To handle the same error as other code returning -E2BIG when the size is too large, it converts return value to -E2BIG. Signed-off-by: Gwan-gyeong Mun Cc: Chris Wilson Cc: Matthew Auld Cc: Thomas Hellström Reviewed-by: Nirmoy Das Reviewed-by: Mauro Carvalho Chehab Reviewed-by: Andrzej Hajda --- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index 6956d273fa5f..955635ae5982 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -1210,6 +1210,17 @@ int __i915_gem_ttm_object_init(struct intel_memory_region *mem, ret = ttm_bo_init_reserved(&i915->bdev, i915_gem_to_ttm(obj), bo_type, &i915_sys_placement, page_size >> PAGE_SHIFT, &ctx, NULL, NULL, i915_ttm_bo_destroy); + + /* + * XXX: The ttm_bo_init_reserved() functions returns -ENOSPC if the size + * is too big to add vma. The direct function that returns -ENOSPC is + * drm_mm_insert_node_in_range(). To handle the same error as other code + * that returns -E2BIG when the size is too large, it converts -ENOSPC to + * -E2BIG. + */ + if (size >> PAGE_SHIFT > INT_MAX && ret == -ENOSPC) + ret = -E2BIG; + if (ret) return i915_ttm_err_to_gem(ret); From patchwork Fri Sep 9 10:59:13 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Gwan-gyeong Mun X-Patchwork-Id: 12971537 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 937ECECAAD3 for ; Fri, 9 Sep 2022 11:00:55 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id BB5BE10EC77; Fri, 9 Sep 2022 11:00:20 +0000 (UTC) Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id F131210EC67; Fri, 9 Sep 2022 11:00:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1662721211; x=1694257211; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=UsK/+r+BKGCTgwY/Wf2xdUgLKrr+HR5kUlhfctdNly0=; b=A29HrTGsx3JyQ5O727jcIk7YeThK3SJH2FoKU6Vamp1zh7Z0tu6K/uMr uL7aO9GLRt5Ay5NqkSK/znvzMpAeUSoNmWFCeQZVv3SNuUNkFY81j7up7 oQ2EagcAGFY8H1cJJjYBd/rWkIjVSveol79vqtFpzB+enYjqu/rCSJ96/ L9YzBFRkAFK+yQ/4DhUbXY1cNos2GCZHXUfBYIR/Br/jlKDDIea1IHBt2 89ixNhBojSs4M6zYqSYqlH0BknTP3+6w8Nkm4hOGQ0j4XzPTCa16SWfOH sjksr6EhL7F0R/0IYErPTx6PsD2X15vQPBtBp2sxlb5LZz837WfIYAmcT A==; X-IronPort-AV: E=McAfee;i="6500,9779,10464"; a="359174729" X-IronPort-AV: E=Sophos;i="5.93,303,1654585200"; d="scan'208";a="359174729" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Sep 2022 04:00:10 -0700 X-IronPort-AV: E=Sophos;i="5.93,303,1654585200"; d="scan'208";a="704370565" Received: from clbarnes-mobl.amr.corp.intel.com (HELO paris.amr.corp.intel.com) ([10.254.42.92]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Sep 2022 04:00:07 -0700 From: Gwan-gyeong Mun To: intel-gfx@lists.freedesktop.org Date: Fri, 9 Sep 2022 19:59:13 +0900 Message-Id: <20220909105913.752049-10-gwan-gyeong.mun@intel.com> X-Mailer: git-send-email 2.37.1 In-Reply-To: <20220909105913.752049-1-gwan-gyeong.mun@intel.com> References: <20220909105913.752049-1-gwan-gyeong.mun@intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH v10 9/9] drm/i915: Remove truncation warning for large objects X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: thomas.hellstrom@linux.intel.com, keescook@chromium.org, jani.nikula@intel.com, ndesaulniers@google.com, dlatypov@google.com, linux@rasmusvillemoes.dk, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, chris@chris-wilson.co.uk, andrzej.hajda@intel.com, matthew.auld@intel.com, daniel@ffwll.ch, airlied@redhat.com, mchehab@kernel.org, vitor@massaru.org, nirmoy.das@intel.com Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" From: Chris Wilson Having addressed the issues surrounding incorrect types for local variables and potential integer truncation in using the scatterlist API, we have closed all the loop holes we had previously identified with dangerously large object creation. As such, we can eliminate the warning put in place to remind us to complete the review. Signed-off-by: Chris Wilson Signed-off-by: Gwan-gyeong Mun Cc: Tvrtko Ursulin Cc: Brian Welty Cc: Matthew Auld Cc: Thomas Hellström Testcase: igt@gem_create@create-massive Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4991 Reviewed-by: Nirmoy Das Reviewed-by: Mauro Carvalho Chehab Reviewed-by: Andrzej Hajda --- drivers/gpu/drm/i915/gem/i915_gem_object.h | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h index a64fe73c05b5..ad88ab88b828 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h @@ -20,25 +20,10 @@ enum intel_region_id; -/* - * XXX: There is a prevalence of the assumption that we fit the - * object's page count inside a 32bit _signed_ variable. Let's document - * this and catch if we ever need to fix it. In the meantime, if you do - * spot such a local variable, please consider fixing! - * - * We can check for invalidly typed locals with typecheck(), see for example - * i915_gem_object_get_sg(). - */ -#define GEM_CHECK_SIZE_OVERFLOW(sz) \ - GEM_WARN_ON((sz) >> PAGE_SHIFT > INT_MAX) - static inline bool i915_gem_object_size_2big(u64 size) { struct drm_i915_gem_object *obj; - if (GEM_CHECK_SIZE_OVERFLOW(size)) - return true; - if (overflows_type(size, obj->base.size)) return true;