From 1ec8d9afc45f783f4609577d3aecb9d21165b8b2 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Sat, 3 Dec 2022 12:23:39 +0100 Subject: [PATCH] 5.15-stable patches added patches: drm-i915-fix-negative-value-passed-as-remaining-time.patch drm-i915-never-return-0-if-not-all-requests-retired.patch tracing-fix-race-where-histograms-can-be-called-before-the-event.patch tracing-free-buffers-when-a-used-dynamic-event-is-removed.patch tracing-osnoise-fix-duration-type.patch --- ...ative-value-passed-as-remaining-time.patch | 65 ++++++ ...return-0-if-not-all-requests-retired.patch | 47 ++++ queue-5.15/series | 5 + ...grams-can-be-called-before-the-event.patch | 48 +++++ ...when-a-used-dynamic-event-is-removed.patch | 204 ++++++++++++++++++ .../tracing-osnoise-fix-duration-type.patch | 59 +++++ 6 files changed, 428 insertions(+) create mode 100644 queue-5.15/drm-i915-fix-negative-value-passed-as-remaining-time.patch create mode 100644 queue-5.15/drm-i915-never-return-0-if-not-all-requests-retired.patch create mode 100644 queue-5.15/tracing-fix-race-where-histograms-can-be-called-before-the-event.patch create mode 100644 queue-5.15/tracing-free-buffers-when-a-used-dynamic-event-is-removed.patch create mode 100644 queue-5.15/tracing-osnoise-fix-duration-type.patch diff --git a/queue-5.15/drm-i915-fix-negative-value-passed-as-remaining-time.patch b/queue-5.15/drm-i915-fix-negative-value-passed-as-remaining-time.patch new file mode 100644 index 00000000000..5a7ef503860 --- /dev/null +++ b/queue-5.15/drm-i915-fix-negative-value-passed-as-remaining-time.patch @@ -0,0 +1,65 @@ +From a8899b8728013c7b2456f0bfa20e5fea85ee0fd1 Mon Sep 17 00:00:00 2001 +From: Janusz Krzysztofik +Date: Mon, 21 Nov 2022 15:56:54 +0100 +Subject: drm/i915: Fix negative value passed as remaining time + +From: Janusz Krzysztofik + +commit a8899b8728013c7b2456f0bfa20e5fea85ee0fd1 upstream. + +Commit b97060a99b01 ("drm/i915/guc: Update intel_gt_wait_for_idle to work +with GuC") extended the API of intel_gt_retire_requests_timeout() with an +extra argument 'remaining_timeout', intended for passing back unconsumed +portion of requested timeout when 0 (success) is returned. However, when +request retirement happens to succeed despite an error returned by a call +to dma_fence_wait_timeout(), that error code (a negative value) is passed +back instead of remaining time. If we then pass that negative value +forward as requested timeout to intel_uc_wait_for_idle(), an explicit BUG +will be triggered. + +If request retirement succeeds but an error code is passed back via +remaininig_timeout, we may have no clue on how much of the initial timeout +might have been left for spending it on waiting for GuC to become idle. +OTOH, since all pending requests have been successfully retired, that +error code has been already ignored by intel_gt_retire_requests_timeout(), +then we shouldn't fail. + +Assume no more time has been left on error and pass 0 timeout value to +intel_uc_wait_for_idle() to give it a chance to return success if GuC is +already idle. + +v3: Don't fail on any error passed back via remaining_timeout. + +v2: Fix the issue on the caller side, not the provider. + +Fixes: b97060a99b01 ("drm/i915/guc: Update intel_gt_wait_for_idle to work with GuC") +Signed-off-by: Janusz Krzysztofik +Cc: stable@vger.kernel.org # v5.15+ +Reviewed-by: Andrzej Hajda +Signed-off-by: Tvrtko Ursulin +Link: https://patchwork.freedesktop.org/patch/msgid/20221121145655.75141-2-janusz.krzysztofik@linux.intel.com +(cherry picked from commit f235dbd5b768e238d365fd05d92de5a32abc1c1f) +Signed-off-by: Tvrtko Ursulin +Signed-off-by: Greg Kroah-Hartman +--- + drivers/gpu/drm/i915/gt/intel_gt.c | 9 +++++++-- + 1 file changed, 7 insertions(+), 2 deletions(-) + +--- a/drivers/gpu/drm/i915/gt/intel_gt.c ++++ b/drivers/gpu/drm/i915/gt/intel_gt.c +@@ -650,8 +650,13 @@ int intel_gt_wait_for_idle(struct intel_ + return -EINTR; + } + +- return timeout ? timeout : intel_uc_wait_for_idle(>->uc, +- remaining_timeout); ++ if (timeout) ++ return timeout; ++ ++ if (remaining_timeout < 0) ++ remaining_timeout = 0; ++ ++ return intel_uc_wait_for_idle(>->uc, remaining_timeout); + } + + int intel_gt_init(struct intel_gt *gt) diff --git a/queue-5.15/drm-i915-never-return-0-if-not-all-requests-retired.patch b/queue-5.15/drm-i915-never-return-0-if-not-all-requests-retired.patch new file mode 100644 index 00000000000..1fc22bbc4ad --- /dev/null +++ b/queue-5.15/drm-i915-never-return-0-if-not-all-requests-retired.patch @@ -0,0 +1,47 @@ +From 12b8b046e4c9de40fa59b6f067d6826f4e688f68 Mon Sep 17 00:00:00 2001 +From: Janusz Krzysztofik +Date: Mon, 21 Nov 2022 15:56:55 +0100 +Subject: drm/i915: Never return 0 if not all requests retired + +From: Janusz Krzysztofik + +commit 12b8b046e4c9de40fa59b6f067d6826f4e688f68 upstream. + +Users of intel_gt_retire_requests_timeout() expect 0 return value on +success. However, we have no protection from passing back 0 potentially +returned by a call to dma_fence_wait_timeout() when it succedes right +after its timeout has expired. + +Replace 0 with -ETIME before potentially using the timeout value as return +code, so -ETIME is returned if there are still some requests not retired +after timeout, 0 otherwise. + +v3: Use conditional expression, more compact but also better reflecting + intention standing behind the change. + +v2: Move the added lines down so flush_submission() is not affected. + +Fixes: f33a8a51602c ("drm/i915: Merge wait_for_timelines with retire_request") +Signed-off-by: Janusz Krzysztofik +Reviewed-by: Andrzej Hajda +Cc: stable@vger.kernel.org # v5.5+ +Signed-off-by: Tvrtko Ursulin +Link: https://patchwork.freedesktop.org/patch/msgid/20221121145655.75141-3-janusz.krzysztofik@linux.intel.com +(cherry picked from commit f301a29f143760ce8d3d6b6a8436d45d3448cde6) +Signed-off-by: Tvrtko Ursulin +Signed-off-by: Greg Kroah-Hartman +--- + drivers/gpu/drm/i915/gt/intel_gt_requests.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +--- a/drivers/gpu/drm/i915/gt/intel_gt_requests.c ++++ b/drivers/gpu/drm/i915/gt/intel_gt_requests.c +@@ -199,7 +199,7 @@ out_active: spin_lock(&timelines->lock); + if (remaining_timeout) + *remaining_timeout = timeout; + +- return active_count ? timeout : 0; ++ return active_count ? timeout ?: -ETIME : 0; + } + + static void retire_work_handler(struct work_struct *work) diff --git a/queue-5.15/series b/queue-5.15/series index fae948a38fc..e6253d6f3c7 100644 --- a/queue-5.15/series +++ b/queue-5.15/series @@ -80,3 +80,8 @@ mmc-sdhci-sprd-fix-no-reset-data-and-command-after-voltage-switch.patch mmc-sdhci-fix-voltage-switch-delay.patch drm-amdgpu-temporarily-disable-broken-clang-builds-due-to-blown-stack-frame.patch drm-amdgpu-enable-vangogh-vcn-indirect-sram-mode.patch +drm-i915-fix-negative-value-passed-as-remaining-time.patch +drm-i915-never-return-0-if-not-all-requests-retired.patch +tracing-osnoise-fix-duration-type.patch +tracing-fix-race-where-histograms-can-be-called-before-the-event.patch +tracing-free-buffers-when-a-used-dynamic-event-is-removed.patch diff --git a/queue-5.15/tracing-fix-race-where-histograms-can-be-called-before-the-event.patch b/queue-5.15/tracing-fix-race-where-histograms-can-be-called-before-the-event.patch new file mode 100644 index 00000000000..a96fd6c403e --- /dev/null +++ b/queue-5.15/tracing-fix-race-where-histograms-can-be-called-before-the-event.patch @@ -0,0 +1,48 @@ +From ef38c79a522b660f7f71d45dad2d6244bc741841 Mon Sep 17 00:00:00 2001 +From: "Steven Rostedt (Google)" +Date: Wed, 23 Nov 2022 16:43:23 -0500 +Subject: tracing: Fix race where histograms can be called before the event + +From: Steven Rostedt (Google) + +commit ef38c79a522b660f7f71d45dad2d6244bc741841 upstream. + +commit 94eedf3dded5 ("tracing: Fix race where eprobes can be called before +the event") fixed an issue where if an event is soft disabled, and the +trigger is being added, there's a small window where the event sees that +there's a trigger but does not see that it requires reading the event yet, +and then calls the trigger with the record == NULL. + +This could be solved with adding memory barriers in the hot path, or to +make sure that all the triggers requiring a record check for NULL. The +latter was chosen. + +Commit 94eedf3dded5 set the eprobe trigger handle to check for NULL, but +the same needs to be done with histograms. + +Link: https://lore.kernel.org/linux-trace-kernel/20221118211809.701d40c0f8a757b0df3c025a@kernel.org/ +Link: https://lore.kernel.org/linux-trace-kernel/20221123164323.03450c3a@gandalf.local.home + +Cc: Tom Zanussi +Cc: stable@vger.kernel.org +Fixes: 7491e2c442781 ("tracing: Add a probe that attaches to trace events") +Reported-by: Masami Hiramatsu (Google) +Acked-by: Masami Hiramatsu (Google) +Signed-off-by: Steven Rostedt (Google) +Signed-off-by: Greg Kroah-Hartman +--- + kernel/trace/trace_events_hist.c | 3 +++ + 1 file changed, 3 insertions(+) + +--- a/kernel/trace/trace_events_hist.c ++++ b/kernel/trace/trace_events_hist.c +@@ -4677,6 +4677,9 @@ static void event_hist_trigger(struct ev + void *key = NULL; + unsigned int i; + ++ if (unlikely(!rbe)) ++ return; ++ + memset(compound_key, 0, hist_data->key_size); + + for_each_hist_key_field(i, hist_data) { diff --git a/queue-5.15/tracing-free-buffers-when-a-used-dynamic-event-is-removed.patch b/queue-5.15/tracing-free-buffers-when-a-used-dynamic-event-is-removed.patch new file mode 100644 index 00000000000..b2dc5536ae6 --- /dev/null +++ b/queue-5.15/tracing-free-buffers-when-a-used-dynamic-event-is-removed.patch @@ -0,0 +1,204 @@ +From 4313e5a613049dfc1819a6dfb5f94cf2caff9452 Mon Sep 17 00:00:00 2001 +From: "Steven Rostedt (Google)" +Date: Wed, 23 Nov 2022 17:14:34 -0500 +Subject: tracing: Free buffers when a used dynamic event is removed +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +From: Steven Rostedt (Google) + +commit 4313e5a613049dfc1819a6dfb5f94cf2caff9452 upstream. + +After 65536 dynamic events have been added and removed, the "type" field +of the event then uses the first type number that is available (not +currently used by other events). A type number is the identifier of the +binary blobs in the tracing ring buffer (known as events) to map them to +logic that can parse the binary blob. + +The issue is that if a dynamic event (like a kprobe event) is traced and +is in the ring buffer, and then that event is removed (because it is +dynamic, which means it can be created and destroyed), if another dynamic +event is created that has the same number that new event's logic on +parsing the binary blob will be used. + +To show how this can be an issue, the following can crash the kernel: + + # cd /sys/kernel/tracing + # for i in `seq 65536`; do + echo 'p:kprobes/foo do_sys_openat2 $arg1:u32' > kprobe_events + # done + +For every iteration of the above, the writing to the kprobe_events will +remove the old event and create a new one (with the same format) and +increase the type number to the next available on until the type number +reaches over 65535 which is the max number for the 16 bit type. After it +reaches that number, the logic to allocate a new number simply looks for +the next available number. When an dynamic event is removed, that number +is then available to be reused by the next dynamic event created. That is, +once the above reaches the max number, the number assigned to the event in +that loop will remain the same. + +Now that means deleting one dynamic event and created another will reuse +the previous events type number. This is where bad things can happen. +After the above loop finishes, the kprobes/foo event which reads the +do_sys_openat2 function call's first parameter as an integer. + + # echo 1 > kprobes/foo/enable + # cat /etc/passwd > /dev/null + # cat trace + cat-2211 [005] .... 2007.849603: foo: (do_sys_openat2+0x0/0x130) arg1=4294967196 + cat-2211 [005] .... 2007.849620: foo: (do_sys_openat2+0x0/0x130) arg1=4294967196 + cat-2211 [005] .... 2007.849838: foo: (do_sys_openat2+0x0/0x130) arg1=4294967196 + cat-2211 [005] .... 2007.849880: foo: (do_sys_openat2+0x0/0x130) arg1=4294967196 + # echo 0 > kprobes/foo/enable + +Now if we delete the kprobe and create a new one that reads a string: + + # echo 'p:kprobes/foo do_sys_openat2 +0($arg2):string' > kprobe_events + +And now we can the trace: + + # cat trace + sendmail-1942 [002] ..... 530.136320: foo: (do_sys_openat2+0x0/0x240) arg1= cat-2046 [004] ..... 530.930817: foo: (do_sys_openat2+0x0/0x240) arg1="������������������������������������������������������������������������������������������������" + cat-2046 [004] ..... 530.930961: foo: (do_sys_openat2+0x0/0x240) arg1="������������������������������������������������������������������������������������������������" + cat-2046 [004] ..... 530.934278: foo: (do_sys_openat2+0x0/0x240) arg1="������������������������������������������������������������������������������������������������" + cat-2046 [004] ..... 530.934563: foo: (do_sys_openat2+0x0/0x240) arg1="������������������������������������������������������������������������������������������������" + bash-1515 [007] ..... 534.299093: foo: (do_sys_openat2+0x0/0x240) arg1="kkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkk���������@��4Z����;Y�����U + +And dmesg has: + +================================================================== +BUG: KASAN: use-after-free in string+0xd4/0x1c0 +Read of size 1 at addr ffff88805fdbbfa0 by task cat/2049 + + CPU: 0 PID: 2049 Comm: cat Not tainted 6.1.0-rc6-test+ #641 + Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v03.03 07/14/2016 + Call Trace: + + dump_stack_lvl+0x5b/0x77 + print_report+0x17f/0x47b + kasan_report+0xad/0x130 + string+0xd4/0x1c0 + vsnprintf+0x500/0x840 + seq_buf_vprintf+0x62/0xc0 + trace_seq_printf+0x10e/0x1e0 + print_type_string+0x90/0xa0 + print_kprobe_event+0x16b/0x290 + print_trace_line+0x451/0x8e0 + s_show+0x72/0x1f0 + seq_read_iter+0x58e/0x750 + seq_read+0x115/0x160 + vfs_read+0x11d/0x460 + ksys_read+0xa9/0x130 + do_syscall_64+0x3a/0x90 + entry_SYSCALL_64_after_hwframe+0x63/0xcd + RIP: 0033:0x7fc2e972ade2 + Code: c0 e9 b2 fe ff ff 50 48 8d 3d b2 3f 0a 00 e8 05 f0 01 00 0f 1f 44 00 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 0f 05 <48> 3d 00 f0 ff ff 77 56 c3 0f 1f 44 00 00 48 83 ec 28 48 89 54 24 + RSP: 002b:00007ffc64e687c8 EFLAGS: 00000246 ORIG_RAX: 0000000000000000 + RAX: ffffffffffffffda RBX: 0000000000020000 RCX: 00007fc2e972ade2 + RDX: 0000000000020000 RSI: 00007fc2e980d000 RDI: 0000000000000003 + RBP: 00007fc2e980d000 R08: 00007fc2e980c010 R09: 0000000000000000 + R10: 0000000000000022 R11: 0000000000000246 R12: 0000000000020f00 + R13: 0000000000000003 R14: 0000000000020000 R15: 0000000000020000 + + + The buggy address belongs to the physical page: + page:ffffea00017f6ec0 refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x5fdbb + flags: 0xfffffc0000000(node=0|zone=1|lastcpupid=0x1fffff) + raw: 000fffffc0000000 0000000000000000 ffffea00017f6ec8 0000000000000000 + raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000 + page dumped because: kasan: bad access detected + + Memory state around the buggy address: + ffff88805fdbbe80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff + ffff88805fdbbf00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff + >ffff88805fdbbf80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff + ^ + ffff88805fdbc000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff + ffff88805fdbc080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff + ================================================================== + +This was found when Zheng Yejian sent a patch to convert the event type +number assignment to use IDA, which gives the next available number, and +this bug showed up in the fuzz testing by Yujie Liu and the kernel test +robot. But after further analysis, I found that this behavior is the same +as when the event type numbers go past the 16bit max (and the above shows +that). + +As modules have a similar issue, but is dealt with by setting a +"WAS_ENABLED" flag when a module event is enabled, and when the module is +freed, if any of its events were enabled, the ring buffer that holds that +event is also cleared, to prevent reading stale events. The same can be +done for dynamic events. + +If any dynamic event that is being removed was enabled, then make sure the +buffers they were enabled in are now cleared. + +Link: https://lkml.kernel.org/r/20221123171434.545706e3@gandalf.local.home +Link: https://lore.kernel.org/all/20221110020319.1259291-1-zhengyejian1@huawei.com/ + +Cc: stable@vger.kernel.org +Cc: Andrew Morton +Depends-on: e18eb8783ec49 ("tracing: Add tracing_reset_all_online_cpus_unlocked() function") +Depends-on: 5448d44c38557 ("tracing: Add unified dynamic event framework") +Depends-on: 6212dd29683ee ("tracing/kprobes: Use dyn_event framework for kprobe events") +Depends-on: 065e63f951432 ("tracing: Only have rmmod clear buffers that its events were active in") +Depends-on: 575380da8b469 ("tracing: Only clear trace buffer on module unload if event was traced") +Fixes: 77b44d1b7c283 ("tracing/kprobes: Rename Kprobe-tracer to kprobe-event") +Reported-by: Zheng Yejian +Reported-by: Yujie Liu +Reported-by: kernel test robot +Acked-by: Masami Hiramatsu (Google) +Signed-off-by: Steven Rostedt (Google) +Signed-off-by: Greg Kroah-Hartman +--- + kernel/trace/trace_dynevent.c | 2 ++ + kernel/trace/trace_events.c | 11 ++++++++++- + 2 files changed, 12 insertions(+), 1 deletion(-) + +--- a/kernel/trace/trace_dynevent.c ++++ b/kernel/trace/trace_dynevent.c +@@ -118,6 +118,7 @@ int dyn_event_release(const char *raw_co + if (ret) + break; + } ++ tracing_reset_all_online_cpus(); + mutex_unlock(&event_mutex); + out: + argv_free(argv); +@@ -214,6 +215,7 @@ int dyn_events_release_all(struct dyn_ev + break; + } + out: ++ tracing_reset_all_online_cpus(); + mutex_unlock(&event_mutex); + + return ret; +--- a/kernel/trace/trace_events.c ++++ b/kernel/trace/trace_events.c +@@ -2874,7 +2874,10 @@ static int probe_remove_event_call(struc + * TRACE_REG_UNREGISTER. + */ + if (file->flags & EVENT_FILE_FL_ENABLED) +- return -EBUSY; ++ goto busy; ++ ++ if (file->flags & EVENT_FILE_FL_WAS_ENABLED) ++ tr->clear_trace = true; + /* + * The do_for_each_event_file_safe() is + * a double loop. After finding the call for this +@@ -2887,6 +2890,12 @@ static int probe_remove_event_call(struc + __trace_remove_event_call(call); + + return 0; ++ busy: ++ /* No need to clear the trace now */ ++ list_for_each_entry(tr, &ftrace_trace_arrays, list) { ++ tr->clear_trace = false; ++ } ++ return -EBUSY; + } + + /* Remove an event_call */ diff --git a/queue-5.15/tracing-osnoise-fix-duration-type.patch b/queue-5.15/tracing-osnoise-fix-duration-type.patch new file mode 100644 index 00000000000..786d925c9ae --- /dev/null +++ b/queue-5.15/tracing-osnoise-fix-duration-type.patch @@ -0,0 +1,59 @@ +From 022632f6c43a86f2135642dccd5686de318e861d Mon Sep 17 00:00:00 2001 +From: Daniel Bristot de Oliveira +Date: Thu, 17 Nov 2022 14:46:17 +0100 +Subject: tracing/osnoise: Fix duration type + +From: Daniel Bristot de Oliveira + +commit 022632f6c43a86f2135642dccd5686de318e861d upstream. + +The duration type is a 64 long value, not an int. This was +causing some long noise to report wrong values. + +Change the duration to a 64 bits value. + +Link: https://lkml.kernel.org/r/a93d8a8378c7973e9c609de05826533c9e977939.1668692096.git.bristot@kernel.org + +Cc: stable@vger.kernel.org +Cc: Daniel Bristot de Oliveira +Cc: Steven Rostedt +Cc: Masami Hiramatsu +Cc: Jonathan Corbet +Fixes: bce29ac9ce0b ("trace: Add osnoise tracer") +Signed-off-by: Daniel Bristot de Oliveira +Acked-by: Masami Hiramatsu (Google) +Signed-off-by: Steven Rostedt (Google) +Signed-off-by: Greg Kroah-Hartman +--- + kernel/trace/trace_osnoise.c | 6 +++--- + 1 file changed, 3 insertions(+), 3 deletions(-) + +--- a/kernel/trace/trace_osnoise.c ++++ b/kernel/trace/trace_osnoise.c +@@ -730,7 +730,7 @@ void osnoise_trace_irq_entry(int id) + void osnoise_trace_irq_exit(int id, const char *desc) + { + struct osnoise_variables *osn_var = this_cpu_osn_var(); +- int duration; ++ s64 duration; + + if (!osn_var->sampling) + return; +@@ -861,7 +861,7 @@ static void trace_softirq_entry_callback + static void trace_softirq_exit_callback(void *data, unsigned int vec_nr) + { + struct osnoise_variables *osn_var = this_cpu_osn_var(); +- int duration; ++ s64 duration; + + if (!osn_var->sampling) + return; +@@ -969,7 +969,7 @@ thread_entry(struct osnoise_variables *o + static void + thread_exit(struct osnoise_variables *osn_var, struct task_struct *t) + { +- int duration; ++ s64 duration; + + if (!osn_var->sampling) + return; -- 2.47.3