Re: [PATCH] drm/xe: Fix global-out-of-bounds in rule_matches

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 01 Jul 2024, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> On Mon, Jul 01, 2024 at 05:37:02PM GMT, Nirmoy Das wrote:
>>Do post-increment instead of pre-increment to fix:
>>[ 9344.404516] BUG: KASAN: global-out-of-bounds in rule_matches+0xb72/0x11c0 [xe]
>>[ 9344.411887] Read of size 1 at addr ffffffffa330b210 by task xe_module_load/248463
>>
>>Fixes: dc72c52a42e0 ("drm/xe/rtp: Allow to OR rules")
>>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>>Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>>---
>> drivers/gpu/drm/xe/xe_rtp.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>>diff --git a/drivers/gpu/drm/xe/xe_rtp.c b/drivers/gpu/drm/xe/xe_rtp.c
>>index 5b27f7c45ea3..f6ec8df5fc94 100644
>>--- a/drivers/gpu/drm/xe/xe_rtp.c
>>+++ b/drivers/gpu/drm/xe/xe_rtp.c
>>@@ -121,7 +121,7 @@ static bool rule_matches(const struct xe_device *xe,
>> 			 * Advance rules until we find XE_RTP_MATCH_OR to check
>> 			 * if there's another set of conditions to check
>> 			 */
>>-			while (i < n_rules && rules[++i].match_type != XE_RTP_MATCH_OR)
>>+			while (i < n_rules && rules[i++].match_type != XE_RTP_MATCH_OR)
>
> this will double check the current iteration. A better fix was posted last
> week and I will merge it soon:

Meh, any messing with loop variables inside a for loop is a footgun. You
need a better abstraction. :p

Even the for loop is just too clever for its own good:

	for (r = rules, i = 0; i < n_rules; r = &rules[++i]) {

This should just be:

	for (i = 0; i < n_rules; i++) {
		r = rules[i];

Don't make it so hard for your future self. ;)

BR,
Jani.

>
> https://patchwork.freedesktop.org/series/135527/
>
> Lucas De Marchi
>
>> 				;
>>
>> 			if (i >= n_rules)
>>-- 
>>2.42.0
>>

-- 
Jani Nikula, Intel