* [PATCH v2] drm/xe: Fix an out-of-bounds shift when invalidating TLB
@ 2025-03-26 15:16 Thomas Hellström
2025-03-27 12:50 ` Thomas Hellström
0 siblings, 1 reply; 3+ messages in thread
From: Thomas Hellström @ 2025-03-26 15:16 UTC (permalink / raw)
To: intel-xe
Cc: Thomas Hellström, Matthew Brost, Rodrigo Vivi, stable,
Lucas De Marchi
When the size of the range invalidated is larger than
rounddown_pow_of_two(ULONG_MAX),
The function macro roundup_pow_of_two(length) will hit an out-of-bounds
shift [1].
Use a full TLB invalidation for such cases.
v2:
- Use a define for the range size limit over which we use a full
TLB invalidation. (Lucas)
- Use a better calculation of the limit.
[1]:
[ 39.202421] ------------[ cut here ]------------
[ 39.202657] UBSAN: shift-out-of-bounds in ./include/linux/log2.h:57:13
[ 39.202673] shift exponent 64 is too large for 64-bit type 'long unsigned int'
[ 39.202688] CPU: 8 UID: 0 PID: 3129 Comm: xe_exec_system_ Tainted: G U 6.14.0+ #10
[ 39.202690] Tainted: [U]=USER
[ 39.202690] Hardware name: ASUS System Product Name/PRIME B560M-A AC, BIOS 2001 02/01/2023
[ 39.202691] Call Trace:
[ 39.202692] <TASK>
[ 39.202695] dump_stack_lvl+0x6e/0xa0
[ 39.202699] ubsan_epilogue+0x5/0x30
[ 39.202701] __ubsan_handle_shift_out_of_bounds.cold+0x61/0xe6
[ 39.202705] xe_gt_tlb_invalidation_range.cold+0x1d/0x3a [xe]
[ 39.202800] ? find_held_lock+0x2b/0x80
[ 39.202803] ? mark_held_locks+0x40/0x70
[ 39.202806] xe_svm_invalidate+0x459/0x700 [xe]
[ 39.202897] drm_gpusvm_notifier_invalidate+0x4d/0x70 [drm_gpusvm]
[ 39.202900] __mmu_notifier_release+0x1f5/0x270
[ 39.202905] exit_mmap+0x40e/0x450
[ 39.202912] __mmput+0x45/0x110
[ 39.202914] exit_mm+0xc5/0x130
[ 39.202916] do_exit+0x21c/0x500
[ 39.202918] ? lockdep_hardirqs_on_prepare+0xdb/0x190
[ 39.202920] do_group_exit+0x36/0xa0
[ 39.202922] get_signal+0x8f8/0x900
[ 39.202926] arch_do_signal_or_restart+0x35/0x100
[ 39.202930] syscall_exit_to_user_mode+0x1fc/0x290
[ 39.202932] do_syscall_64+0xa1/0x180
[ 39.202934] ? do_user_addr_fault+0x59f/0x8a0
[ 39.202937] ? lock_release+0xd2/0x2a0
[ 39.202939] ? do_user_addr_fault+0x5a9/0x8a0
[ 39.202942] ? trace_hardirqs_off+0x4b/0xc0
[ 39.202944] ? clear_bhb_loop+0x25/0x80
[ 39.202946] ? clear_bhb_loop+0x25/0x80
[ 39.202947] ? clear_bhb_loop+0x25/0x80
[ 39.202950] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 39.202952] RIP: 0033:0x7fa945e543e1
[ 39.202961] Code: Unable to access opcode bytes at 0x7fa945e543b7.
[ 39.202962] RSP: 002b:00007ffca8fb4170 EFLAGS: 00000293
[ 39.202963] RAX: 000000000000003d RBX: 0000000000000000 RCX: 00007fa945e543e3
[ 39.202964] RDX: 0000000000000000 RSI: 00007ffca8fb41ac RDI: 00000000ffffffff
[ 39.202964] RBP: 00007ffca8fb4190 R08: 0000000000000000 R09: 00007fa945f600a0
[ 39.202965] R10: 0000000000000000 R11: 0000000000000293 R12: 0000000000000000
[ 39.202966] R13: 00007fa9460dd310 R14: 00007ffca8fb41ac R15: 0000000000000000
[ 39.202970] </TASK>
[ 39.202970] ---[ end trace ]---
Fixes: 332dd0116c82 ("drm/xe: Add range based TLB invalidations")
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: <stable@vger.kernel.org> # v6.8+
Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> #v1
---
drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c b/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
index 03072e094991..084cbdeba8ea 100644
--- a/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
+++ b/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
@@ -322,6 +322,13 @@ int xe_gt_tlb_invalidation_ggtt(struct xe_gt *gt)
return 0;
}
+/*
+ * Ensure that roundup_pow_of_two(length) doesn't overflow.
+ * Note that roundup_pow_of_two() operates on unsigned long,
+ * not on u64.
+ */
+#define MAX_RANGE_TLB_INVALIDATION_LENGTH (rounddown_pow_of_two(ULONG_MAX))
+
/**
* xe_gt_tlb_invalidation_range - Issue a TLB invalidation on this GT for an
* address range
@@ -346,6 +353,7 @@ int xe_gt_tlb_invalidation_range(struct xe_gt *gt,
struct xe_device *xe = gt_to_xe(gt);
#define MAX_TLB_INVALIDATION_LEN 7
u32 action[MAX_TLB_INVALIDATION_LEN];
+ u64 length = end - start;
int len = 0;
xe_gt_assert(gt, fence);
@@ -358,11 +366,11 @@ int xe_gt_tlb_invalidation_range(struct xe_gt *gt,
action[len++] = XE_GUC_ACTION_TLB_INVALIDATION;
action[len++] = 0; /* seqno, replaced in send_tlb_invalidation */
- if (!xe->info.has_range_tlb_invalidation) {
+ if (!xe->info.has_range_tlb_invalidation ||
+ length > MAX_RANGE_TLB_INVALIDATION_LENGTH) {
action[len++] = MAKE_INVAL_OP(XE_GUC_TLB_INVAL_FULL);
} else {
u64 orig_start = start;
- u64 length = end - start;
u64 align;
if (length < SZ_4K)
--
2.48.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] drm/xe: Fix an out-of-bounds shift when invalidating TLB
2025-03-26 15:16 [PATCH v2] drm/xe: Fix an out-of-bounds shift when invalidating TLB Thomas Hellström
@ 2025-03-27 12:50 ` Thomas Hellström
2025-03-27 14:30 ` Lucas De Marchi
0 siblings, 1 reply; 3+ messages in thread
From: Thomas Hellström @ 2025-03-27 12:50 UTC (permalink / raw)
To: intel-xe, Lucas De Marchi
Cc: Matthew Brost, Rodrigo Vivi, stable, Lucas De Marchi
Hi, Lucas,
On Wed, 2025-03-26 at 16:16 +0100, Thomas Hellström wrote:
> When the size of the range invalidated is larger than
> rounddown_pow_of_two(ULONG_MAX),
> The function macro roundup_pow_of_two(length) will hit an out-of-
> bounds
> shift [1].
>
> Use a full TLB invalidation for such cases.
> v2:
> - Use a define for the range size limit over which we use a full
> TLB invalidation. (Lucas)
> - Use a better calculation of the limit.
Does your R-B hold also for v2?
Thanks,
Thomas
>
> [1]:
> [ 39.202421] ------------[ cut here ]------------
> [ 39.202657] UBSAN: shift-out-of-bounds in
> ./include/linux/log2.h:57:13
> [ 39.202673] shift exponent 64 is too large for 64-bit type 'long
> unsigned int'
> [ 39.202688] CPU: 8 UID: 0 PID: 3129 Comm: xe_exec_system_ Tainted:
> G U 6.14.0+ #10
> [ 39.202690] Tainted: [U]=USER
> [ 39.202690] Hardware name: ASUS System Product Name/PRIME B560M-A
> AC, BIOS 2001 02/01/2023
> [ 39.202691] Call Trace:
> [ 39.202692] <TASK>
> [ 39.202695] dump_stack_lvl+0x6e/0xa0
> [ 39.202699] ubsan_epilogue+0x5/0x30
> [ 39.202701] __ubsan_handle_shift_out_of_bounds.cold+0x61/0xe6
> [ 39.202705] xe_gt_tlb_invalidation_range.cold+0x1d/0x3a [xe]
> [ 39.202800] ? find_held_lock+0x2b/0x80
> [ 39.202803] ? mark_held_locks+0x40/0x70
> [ 39.202806] xe_svm_invalidate+0x459/0x700 [xe]
> [ 39.202897] drm_gpusvm_notifier_invalidate+0x4d/0x70 [drm_gpusvm]
> [ 39.202900] __mmu_notifier_release+0x1f5/0x270
> [ 39.202905] exit_mmap+0x40e/0x450
> [ 39.202912] __mmput+0x45/0x110
> [ 39.202914] exit_mm+0xc5/0x130
> [ 39.202916] do_exit+0x21c/0x500
> [ 39.202918] ? lockdep_hardirqs_on_prepare+0xdb/0x190
> [ 39.202920] do_group_exit+0x36/0xa0
> [ 39.202922] get_signal+0x8f8/0x900
> [ 39.202926] arch_do_signal_or_restart+0x35/0x100
> [ 39.202930] syscall_exit_to_user_mode+0x1fc/0x290
> [ 39.202932] do_syscall_64+0xa1/0x180
> [ 39.202934] ? do_user_addr_fault+0x59f/0x8a0
> [ 39.202937] ? lock_release+0xd2/0x2a0
> [ 39.202939] ? do_user_addr_fault+0x5a9/0x8a0
> [ 39.202942] ? trace_hardirqs_off+0x4b/0xc0
> [ 39.202944] ? clear_bhb_loop+0x25/0x80
> [ 39.202946] ? clear_bhb_loop+0x25/0x80
> [ 39.202947] ? clear_bhb_loop+0x25/0x80
> [ 39.202950] entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [ 39.202952] RIP: 0033:0x7fa945e543e1
> [ 39.202961] Code: Unable to access opcode bytes at 0x7fa945e543b7.
> [ 39.202962] RSP: 002b:00007ffca8fb4170 EFLAGS: 00000293
> [ 39.202963] RAX: 000000000000003d RBX: 0000000000000000 RCX:
> 00007fa945e543e3
> [ 39.202964] RDX: 0000000000000000 RSI: 00007ffca8fb41ac RDI:
> 00000000ffffffff
> [ 39.202964] RBP: 00007ffca8fb4190 R08: 0000000000000000 R09:
> 00007fa945f600a0
> [ 39.202965] R10: 0000000000000000 R11: 0000000000000293 R12:
> 0000000000000000
> [ 39.202966] R13: 00007fa9460dd310 R14: 00007ffca8fb41ac R15:
> 0000000000000000
> [ 39.202970] </TASK>
> [ 39.202970] ---[ end trace ]---
>
> Fixes: 332dd0116c82 ("drm/xe: Add range based TLB invalidations")
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: <stable@vger.kernel.org> # v6.8+
> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> #v1
> ---
> drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
> b/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
> index 03072e094991..084cbdeba8ea 100644
> --- a/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
> +++ b/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
> @@ -322,6 +322,13 @@ int xe_gt_tlb_invalidation_ggtt(struct xe_gt
> *gt)
> return 0;
> }
>
> +/*
> + * Ensure that roundup_pow_of_two(length) doesn't overflow.
> + * Note that roundup_pow_of_two() operates on unsigned long,
> + * not on u64.
> + */
> +#define MAX_RANGE_TLB_INVALIDATION_LENGTH
> (rounddown_pow_of_two(ULONG_MAX))
> +
> /**
> * xe_gt_tlb_invalidation_range - Issue a TLB invalidation on this
> GT for an
> * address range
> @@ -346,6 +353,7 @@ int xe_gt_tlb_invalidation_range(struct xe_gt
> *gt,
> struct xe_device *xe = gt_to_xe(gt);
> #define MAX_TLB_INVALIDATION_LEN 7
> u32 action[MAX_TLB_INVALIDATION_LEN];
> + u64 length = end - start;
> int len = 0;
>
> xe_gt_assert(gt, fence);
> @@ -358,11 +366,11 @@ int xe_gt_tlb_invalidation_range(struct xe_gt
> *gt,
>
> action[len++] = XE_GUC_ACTION_TLB_INVALIDATION;
> action[len++] = 0; /* seqno, replaced in
> send_tlb_invalidation */
> - if (!xe->info.has_range_tlb_invalidation) {
> + if (!xe->info.has_range_tlb_invalidation ||
> + length > MAX_RANGE_TLB_INVALIDATION_LENGTH) {
> action[len++] =
> MAKE_INVAL_OP(XE_GUC_TLB_INVAL_FULL);
> } else {
> u64 orig_start = start;
> - u64 length = end - start;
> u64 align;
>
> if (length < SZ_4K)
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] drm/xe: Fix an out-of-bounds shift when invalidating TLB
2025-03-27 12:50 ` Thomas Hellström
@ 2025-03-27 14:30 ` Lucas De Marchi
0 siblings, 0 replies; 3+ messages in thread
From: Lucas De Marchi @ 2025-03-27 14:30 UTC (permalink / raw)
To: Thomas Hellström; +Cc: intel-xe, Matthew Brost, Rodrigo Vivi, stable
On Thu, Mar 27, 2025 at 01:50:43PM +0100, Thomas Hellström wrote:
>Hi, Lucas,
>
>On Wed, 2025-03-26 at 16:16 +0100, Thomas Hellström wrote:
>> When the size of the range invalidated is larger than
>> rounddown_pow_of_two(ULONG_MAX),
>> The function macro roundup_pow_of_two(length) will hit an out-of-
>> bounds
>> shift [1].
>>
>> Use a full TLB invalidation for such cases.
>> v2:
>> - Use a define for the range size limit over which we use a full
>> TLB invalidation. (Lucas)
>> - Use a better calculation of the limit.
>
>Does your R-B hold also for v2?
yes. I double checked rounddown_pow_of_two() would do the right thing
for a constant value and it lgtm.
thanks
Lucas De Marchi
>
>Thanks,
>Thomas
>
>
>>
>> [1]:
>> [ 39.202421] ------------[ cut here ]------------
>> [ 39.202657] UBSAN: shift-out-of-bounds in
>> ./include/linux/log2.h:57:13
>> [ 39.202673] shift exponent 64 is too large for 64-bit type 'long
>> unsigned int'
>> [ 39.202688] CPU: 8 UID: 0 PID: 3129 Comm: xe_exec_system_ Tainted:
>> G U 6.14.0+ #10
>> [ 39.202690] Tainted: [U]=USER
>> [ 39.202690] Hardware name: ASUS System Product Name/PRIME B560M-A
>> AC, BIOS 2001 02/01/2023
>> [ 39.202691] Call Trace:
>> [ 39.202692] <TASK>
>> [ 39.202695] dump_stack_lvl+0x6e/0xa0
>> [ 39.202699] ubsan_epilogue+0x5/0x30
>> [ 39.202701] __ubsan_handle_shift_out_of_bounds.cold+0x61/0xe6
>> [ 39.202705] xe_gt_tlb_invalidation_range.cold+0x1d/0x3a [xe]
>> [ 39.202800] ? find_held_lock+0x2b/0x80
>> [ 39.202803] ? mark_held_locks+0x40/0x70
>> [ 39.202806] xe_svm_invalidate+0x459/0x700 [xe]
>> [ 39.202897] drm_gpusvm_notifier_invalidate+0x4d/0x70 [drm_gpusvm]
>> [ 39.202900] __mmu_notifier_release+0x1f5/0x270
>> [ 39.202905] exit_mmap+0x40e/0x450
>> [ 39.202912] __mmput+0x45/0x110
>> [ 39.202914] exit_mm+0xc5/0x130
>> [ 39.202916] do_exit+0x21c/0x500
>> [ 39.202918] ? lockdep_hardirqs_on_prepare+0xdb/0x190
>> [ 39.202920] do_group_exit+0x36/0xa0
>> [ 39.202922] get_signal+0x8f8/0x900
>> [ 39.202926] arch_do_signal_or_restart+0x35/0x100
>> [ 39.202930] syscall_exit_to_user_mode+0x1fc/0x290
>> [ 39.202932] do_syscall_64+0xa1/0x180
>> [ 39.202934] ? do_user_addr_fault+0x59f/0x8a0
>> [ 39.202937] ? lock_release+0xd2/0x2a0
>> [ 39.202939] ? do_user_addr_fault+0x5a9/0x8a0
>> [ 39.202942] ? trace_hardirqs_off+0x4b/0xc0
>> [ 39.202944] ? clear_bhb_loop+0x25/0x80
>> [ 39.202946] ? clear_bhb_loop+0x25/0x80
>> [ 39.202947] ? clear_bhb_loop+0x25/0x80
>> [ 39.202950] entry_SYSCALL_64_after_hwframe+0x76/0x7e
>> [ 39.202952] RIP: 0033:0x7fa945e543e1
>> [ 39.202961] Code: Unable to access opcode bytes at 0x7fa945e543b7.
>> [ 39.202962] RSP: 002b:00007ffca8fb4170 EFLAGS: 00000293
>> [ 39.202963] RAX: 000000000000003d RBX: 0000000000000000 RCX:
>> 00007fa945e543e3
>> [ 39.202964] RDX: 0000000000000000 RSI: 00007ffca8fb41ac RDI:
>> 00000000ffffffff
>> [ 39.202964] RBP: 00007ffca8fb4190 R08: 0000000000000000 R09:
>> 00007fa945f600a0
>> [ 39.202965] R10: 0000000000000000 R11: 0000000000000293 R12:
>> 0000000000000000
>> [ 39.202966] R13: 00007fa9460dd310 R14: 00007ffca8fb41ac R15:
>> 0000000000000000
>> [ 39.202970] </TASK>
>> [ 39.202970] ---[ end trace ]---
>>
>> Fixes: 332dd0116c82 ("drm/xe: Add range based TLB invalidations")
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: <stable@vger.kernel.org> # v6.8+
>> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> #v1
>> ---
>> drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c | 12 ++++++++++--
>> 1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
>> b/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
>> index 03072e094991..084cbdeba8ea 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
>> +++ b/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
>> @@ -322,6 +322,13 @@ int xe_gt_tlb_invalidation_ggtt(struct xe_gt
>> *gt)
>> return 0;
>> }
>>
>> +/*
>> + * Ensure that roundup_pow_of_two(length) doesn't overflow.
>> + * Note that roundup_pow_of_two() operates on unsigned long,
>> + * not on u64.
>> + */
>> +#define MAX_RANGE_TLB_INVALIDATION_LENGTH
>> (rounddown_pow_of_two(ULONG_MAX))
>> +
>> /**
>> * xe_gt_tlb_invalidation_range - Issue a TLB invalidation on this
>> GT for an
>> * address range
>> @@ -346,6 +353,7 @@ int xe_gt_tlb_invalidation_range(struct xe_gt
>> *gt,
>> struct xe_device *xe = gt_to_xe(gt);
>> #define MAX_TLB_INVALIDATION_LEN 7
>> u32 action[MAX_TLB_INVALIDATION_LEN];
>> + u64 length = end - start;
>> int len = 0;
>>
>> xe_gt_assert(gt, fence);
>> @@ -358,11 +366,11 @@ int xe_gt_tlb_invalidation_range(struct xe_gt
>> *gt,
>>
>> action[len++] = XE_GUC_ACTION_TLB_INVALIDATION;
>> action[len++] = 0; /* seqno, replaced in
>> send_tlb_invalidation */
>> - if (!xe->info.has_range_tlb_invalidation) {
>> + if (!xe->info.has_range_tlb_invalidation ||
>> + length > MAX_RANGE_TLB_INVALIDATION_LENGTH) {
>> action[len++] =
>> MAKE_INVAL_OP(XE_GUC_TLB_INVAL_FULL);
>> } else {
>> u64 orig_start = start;
>> - u64 length = end - start;
>> u64 align;
>>
>> if (length < SZ_4K)
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-03-27 14:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-26 15:16 [PATCH v2] drm/xe: Fix an out-of-bounds shift when invalidating TLB Thomas Hellström
2025-03-27 12:50 ` Thomas Hellström
2025-03-27 14:30 ` Lucas De Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).