From 88c5ffa9bc4798e4586f70cf4b1db0a377d7102b Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Mon, 9 Aug 2021 11:22:22 +0200 Subject: [PATCH] 5.13-stable patches added patches: arm64-stacktrace-avoid-tracing-arch_stack_walk.patch clk-fix-leak-on-devm_clk_bulk_get_all-unwind.patch optee-clear-stale-cache-entries-during-initialization.patch optee-fix-memory-leak-when-failing-to-register-shm-pages.patch optee-fix-tee-out-of-memory-failure-seen-during-kexec-reboot.patch optee-refuse-to-load-the-driver-under-the-kdump-kernel.patch scripts-tracing-fix-the-bug-that-can-t-parse-raw_trace_func.patch tee-add-tee_shm_alloc_kernel_buf.patch tee-correct-inappropriate-usage-of-tee_shm_dma_buf-flag.patch tpm_ftpm_tee-free-and-unregister-tee-shared-memory-during-kexec.patch tracepoint-fix-static-call-function-vs-data-state-mismatch.patch tracepoint-static-call-compare-data-on-transition-from-2-1-callees.patch tracepoint-use-rcu-get-state-and-cond-sync-for-static-call-updates.patch tracing-fix-null-pointer-dereference-in-start_creating.patch tracing-histogram-give-calculation-hist_fields-a-size.patch tracing-reject-string-operand-in-the-histogram-expression.patch --- ...ktrace-avoid-tracing-arch_stack_walk.patch | 133 ++++++++++ ...leak-on-devm_clk_bulk_get_all-unwind.patch | 73 ++++++ ...-cache-entries-during-initialization.patch | 121 ++++++++++ ...k-when-failing-to-register-shm-pages.patch | 54 +++++ ...ory-failure-seen-during-kexec-reboot.patch | 77 ++++++ ...ad-the-driver-under-the-kdump-kernel.patch | 87 +++++++ ...-bug-that-can-t-parse-raw_trace_func.patch | 66 +++++ queue-5.13/series | 16 ++ .../tee-add-tee_shm_alloc_kernel_buf.patch | 60 +++++ ...priate-usage-of-tee_shm_dma_buf-flag.patch | 133 ++++++++++ ...ister-tee-shared-memory-during-kexec.patch | 53 ++++ ...call-function-vs-data-state-mismatch.patch | 228 ++++++++++++++++++ ...-data-on-transition-from-2-1-callees.patch | 42 ++++ ...nd-cond-sync-for-static-call-updates.patch | 185 ++++++++++++++ ...ointer-dereference-in-start_creating.patch | 47 ++++ ...-give-calculation-hist_fields-a-size.patch | 65 +++++ ...-operand-in-the-histogram-expression.patch | 74 ++++++ 17 files changed, 1514 insertions(+) create mode 100644 queue-5.13/arm64-stacktrace-avoid-tracing-arch_stack_walk.patch create mode 100644 queue-5.13/clk-fix-leak-on-devm_clk_bulk_get_all-unwind.patch create mode 100644 queue-5.13/optee-clear-stale-cache-entries-during-initialization.patch create mode 100644 queue-5.13/optee-fix-memory-leak-when-failing-to-register-shm-pages.patch create mode 100644 queue-5.13/optee-fix-tee-out-of-memory-failure-seen-during-kexec-reboot.patch create mode 100644 queue-5.13/optee-refuse-to-load-the-driver-under-the-kdump-kernel.patch create mode 100644 queue-5.13/scripts-tracing-fix-the-bug-that-can-t-parse-raw_trace_func.patch create mode 100644 queue-5.13/tee-add-tee_shm_alloc_kernel_buf.patch create mode 100644 queue-5.13/tee-correct-inappropriate-usage-of-tee_shm_dma_buf-flag.patch create mode 100644 queue-5.13/tpm_ftpm_tee-free-and-unregister-tee-shared-memory-during-kexec.patch create mode 100644 queue-5.13/tracepoint-fix-static-call-function-vs-data-state-mismatch.patch create mode 100644 queue-5.13/tracepoint-static-call-compare-data-on-transition-from-2-1-callees.patch create mode 100644 queue-5.13/tracepoint-use-rcu-get-state-and-cond-sync-for-static-call-updates.patch create mode 100644 queue-5.13/tracing-fix-null-pointer-dereference-in-start_creating.patch create mode 100644 queue-5.13/tracing-histogram-give-calculation-hist_fields-a-size.patch create mode 100644 queue-5.13/tracing-reject-string-operand-in-the-histogram-expression.patch diff --git a/queue-5.13/arm64-stacktrace-avoid-tracing-arch_stack_walk.patch b/queue-5.13/arm64-stacktrace-avoid-tracing-arch_stack_walk.patch new file mode 100644 index 00000000000..ec4de61fb2c --- /dev/null +++ b/queue-5.13/arm64-stacktrace-avoid-tracing-arch_stack_walk.patch @@ -0,0 +1,133 @@ +From 0c32706dac1b0a72713184246952ab0f54327c21 Mon Sep 17 00:00:00 2001 +From: Mark Rutland +Date: Mon, 2 Aug 2021 17:48:45 +0100 +Subject: arm64: stacktrace: avoid tracing arch_stack_walk() + +From: Mark Rutland + +commit 0c32706dac1b0a72713184246952ab0f54327c21 upstream. + +When the function_graph tracer is in use, arch_stack_walk() may unwind +the stack incorrectly, erroneously reporting itself, missing the final +entry which is being traced, and reporting all traced entries between +these off-by-one from where they should be. + +When ftrace hooks a function return, the original return address is +saved to the fgraph ret_stack, and the return address in the LR (or the +function's frame record) is replaced with `return_to_handler`. + +When arm64's unwinder encounter frames returning to `return_to_handler`, +it finds the associated original return address from the fgraph ret +stack, assuming the most recent `ret_to_hander` entry on the stack +corresponds to the most recent entry in the fgraph ret stack, and so on. + +When arch_stack_walk() is used to dump the current task's stack, it +starts from the caller of arch_stack_walk(). However, arch_stack_walk() +can be traced, and so may push an entry on to the fgraph ret stack, +leaving the fgraph ret stack offset by one from the expected position. + +This can be seen when dumping the stack via /proc/self/stack, where +enabling the graph tracer results in an unexpected +`stack_trace_save_tsk` entry at the start of the trace, and `el0_svc` +missing form the end of the trace. + +This patch fixes this by marking arch_stack_walk() as notrace, as we do +for all other functions on the path to ftrace_graph_get_ret_stack(). +While a few helper functions are not marked notrace, their calls/returns +are balanced, and will have no observable effect when examining the +fgraph ret stack. + +It is possible for an exeption boundary to cause a similar offset if the +return address of the interrupted context was in the LR. Fixing those +cases will require some more substantial rework, and is left for +subsequent patches. + +Before: + +| # cat /proc/self/stack +| [<0>] proc_pid_stack+0xc4/0x140 +| [<0>] proc_single_show+0x6c/0x120 +| [<0>] seq_read_iter+0x240/0x4e0 +| [<0>] seq_read+0xe8/0x140 +| [<0>] vfs_read+0xb8/0x1e4 +| [<0>] ksys_read+0x74/0x100 +| [<0>] __arm64_sys_read+0x28/0x3c +| [<0>] invoke_syscall+0x50/0x120 +| [<0>] el0_svc_common.constprop.0+0xc4/0xd4 +| [<0>] do_el0_svc+0x30/0x9c +| [<0>] el0_svc+0x2c/0x54 +| [<0>] el0t_64_sync_handler+0x1a8/0x1b0 +| [<0>] el0t_64_sync+0x198/0x19c +| # echo function_graph > /sys/kernel/tracing/current_tracer +| # cat /proc/self/stack +| [<0>] stack_trace_save_tsk+0xa4/0x110 +| [<0>] proc_pid_stack+0xc4/0x140 +| [<0>] proc_single_show+0x6c/0x120 +| [<0>] seq_read_iter+0x240/0x4e0 +| [<0>] seq_read+0xe8/0x140 +| [<0>] vfs_read+0xb8/0x1e4 +| [<0>] ksys_read+0x74/0x100 +| [<0>] __arm64_sys_read+0x28/0x3c +| [<0>] invoke_syscall+0x50/0x120 +| [<0>] el0_svc_common.constprop.0+0xc4/0xd4 +| [<0>] do_el0_svc+0x30/0x9c +| [<0>] el0t_64_sync_handler+0x1a8/0x1b0 +| [<0>] el0t_64_sync+0x198/0x19c + +After: + +| # cat /proc/self/stack +| [<0>] proc_pid_stack+0xc4/0x140 +| [<0>] proc_single_show+0x6c/0x120 +| [<0>] seq_read_iter+0x240/0x4e0 +| [<0>] seq_read+0xe8/0x140 +| [<0>] vfs_read+0xb8/0x1e4 +| [<0>] ksys_read+0x74/0x100 +| [<0>] __arm64_sys_read+0x28/0x3c +| [<0>] invoke_syscall+0x50/0x120 +| [<0>] el0_svc_common.constprop.0+0xc4/0xd4 +| [<0>] do_el0_svc+0x30/0x9c +| [<0>] el0_svc+0x2c/0x54 +| [<0>] el0t_64_sync_handler+0x1a8/0x1b0 +| [<0>] el0t_64_sync+0x198/0x19c +| # echo function_graph > /sys/kernel/tracing/current_tracer +| # cat /proc/self/stack +| [<0>] proc_pid_stack+0xc4/0x140 +| [<0>] proc_single_show+0x6c/0x120 +| [<0>] seq_read_iter+0x240/0x4e0 +| [<0>] seq_read+0xe8/0x140 +| [<0>] vfs_read+0xb8/0x1e4 +| [<0>] ksys_read+0x74/0x100 +| [<0>] __arm64_sys_read+0x28/0x3c +| [<0>] invoke_syscall+0x50/0x120 +| [<0>] el0_svc_common.constprop.0+0xc4/0xd4 +| [<0>] do_el0_svc+0x30/0x9c +| [<0>] el0_svc+0x2c/0x54 +| [<0>] el0t_64_sync_handler+0x1a8/0x1b0 +| [<0>] el0t_64_sync+0x198/0x19c + +Cc: +Signed-off-by: Mark Rutland +Cc: Catalin Marinas +Cc: Madhavan T. Venkataraman +Cc: Mark Brown +Cc: Will Deacon +Reviwed-by: Mark Brown +Link: https://lore.kernel.org/r/20210802164845.45506-3-mark.rutland@arm.com +Signed-off-by: Will Deacon +Signed-off-by: Greg Kroah-Hartman +--- + arch/arm64/kernel/stacktrace.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +--- a/arch/arm64/kernel/stacktrace.c ++++ b/arch/arm64/kernel/stacktrace.c +@@ -220,7 +220,7 @@ void show_stack(struct task_struct *tsk, + + #ifdef CONFIG_STACKTRACE + +-noinline void arch_stack_walk(stack_trace_consume_fn consume_entry, ++noinline notrace void arch_stack_walk(stack_trace_consume_fn consume_entry, + void *cookie, struct task_struct *task, + struct pt_regs *regs) + { diff --git a/queue-5.13/clk-fix-leak-on-devm_clk_bulk_get_all-unwind.patch b/queue-5.13/clk-fix-leak-on-devm_clk_bulk_get_all-unwind.patch new file mode 100644 index 00000000000..e63bf46bac7 --- /dev/null +++ b/queue-5.13/clk-fix-leak-on-devm_clk_bulk_get_all-unwind.patch @@ -0,0 +1,73 @@ +From f828b0bcacef189edbd247e9f48864fc36bfbe33 Mon Sep 17 00:00:00 2001 +From: Brian Norris +Date: Fri, 30 Jul 2021 19:59:50 -0700 +Subject: clk: fix leak on devm_clk_bulk_get_all() unwind + +From: Brian Norris + +commit f828b0bcacef189edbd247e9f48864fc36bfbe33 upstream. + +clk_bulk_get_all() allocates an array of struct clk_bulk data for us +(unlike clk_bulk_get()), so we need to free it. Let's use the +clk_bulk_put_all() helper. + +kmemleak complains, on an RK3399 Gru/Kevin system: + +unreferenced object 0xffffff80045def00 (size 128): + comm "swapper/0", pid 1, jiffies 4294667682 (age 86.394s) + hex dump (first 32 bytes): + 44 32 60 fe fe ff ff ff 00 00 00 00 00 00 00 00 D2`............. + 48 32 60 fe fe ff ff ff 00 00 00 00 00 00 00 00 H2`............. + backtrace: + [<00000000742860d6>] __kmalloc+0x22c/0x39c + [<00000000b0493f2c>] clk_bulk_get_all+0x64/0x188 + [<00000000325f5900>] devm_clk_bulk_get_all+0x58/0xa8 + [<00000000175b9bc5>] dwc3_probe+0x8ac/0xb5c + [<000000009169e2f9>] platform_drv_probe+0x9c/0xbc + [<000000005c51e2ee>] really_probe+0x13c/0x378 + [<00000000c47b1f24>] driver_probe_device+0x84/0xc0 + [<00000000f870fcfb>] __device_attach_driver+0x94/0xb0 + [<000000004d1b92ae>] bus_for_each_drv+0x8c/0xd8 + [<00000000481d60c3>] __device_attach+0xc4/0x150 + [<00000000a163bd36>] device_initial_probe+0x1c/0x28 + [<00000000accb6bad>] bus_probe_device+0x3c/0x9c + [<000000001a199f89>] device_add+0x218/0x3cc + [<000000001bd84952>] of_device_add+0x40/0x50 + [<000000009c658c29>] of_platform_device_create_pdata+0xac/0x100 + [<0000000021c69ba4>] of_platform_bus_create+0x190/0x224 + +Fixes: f08c2e2865f6 ("clk: add managed version of clk_bulk_get_all") +Cc: Dong Aisheng +Cc: stable@vger.kernel.org +Signed-off-by: Brian Norris +Link: https://lore.kernel.org/r/20210731025950.2238582-1-briannorris@chromium.org +Signed-off-by: Stephen Boyd +Signed-off-by: Greg Kroah-Hartman +--- + drivers/clk/clk-devres.c | 9 ++++++++- + 1 file changed, 8 insertions(+), 1 deletion(-) + +--- a/drivers/clk/clk-devres.c ++++ b/drivers/clk/clk-devres.c +@@ -92,13 +92,20 @@ int __must_check devm_clk_bulk_get_optio + } + EXPORT_SYMBOL_GPL(devm_clk_bulk_get_optional); + ++static void devm_clk_bulk_release_all(struct device *dev, void *res) ++{ ++ struct clk_bulk_devres *devres = res; ++ ++ clk_bulk_put_all(devres->num_clks, devres->clks); ++} ++ + int __must_check devm_clk_bulk_get_all(struct device *dev, + struct clk_bulk_data **clks) + { + struct clk_bulk_devres *devres; + int ret; + +- devres = devres_alloc(devm_clk_bulk_release, ++ devres = devres_alloc(devm_clk_bulk_release_all, + sizeof(*devres), GFP_KERNEL); + if (!devres) + return -ENOMEM; diff --git a/queue-5.13/optee-clear-stale-cache-entries-during-initialization.patch b/queue-5.13/optee-clear-stale-cache-entries-during-initialization.patch new file mode 100644 index 00000000000..1f41b699915 --- /dev/null +++ b/queue-5.13/optee-clear-stale-cache-entries-during-initialization.patch @@ -0,0 +1,121 @@ +From b5c10dd04b7418793517e3286cde5c04759a86de Mon Sep 17 00:00:00 2001 +From: Tyler Hicks +Date: Mon, 14 Jun 2021 17:33:13 -0500 +Subject: optee: Clear stale cache entries during initialization + +From: Tyler Hicks + +commit b5c10dd04b7418793517e3286cde5c04759a86de upstream. + +The shm cache could contain invalid addresses if +optee_disable_shm_cache() was not called from the .shutdown hook of the +previous kernel before a kexec. These addresses could be unmapped or +they could point to mapped but unintended locations in memory. + +Clear the shared memory cache, while being careful to not translate the +addresses returned from OPTEE_SMC_DISABLE_SHM_CACHE, during driver +initialization. Once all pre-cache shm objects are removed, proceed with +enabling the cache so that we know that we can handle cached shm objects +with confidence later in the .shutdown hook. + +Cc: stable@vger.kernel.org +Signed-off-by: Tyler Hicks +Reviewed-by: Jens Wiklander +Reviewed-by: Sumit Garg +Signed-off-by: Jens Wiklander +Signed-off-by: Greg Kroah-Hartman +--- + drivers/tee/optee/call.c | 36 +++++++++++++++++++++++++++++++++--- + drivers/tee/optee/core.c | 9 +++++++++ + drivers/tee/optee/optee_private.h | 1 + + 3 files changed, 43 insertions(+), 3 deletions(-) + +--- a/drivers/tee/optee/call.c ++++ b/drivers/tee/optee/call.c +@@ -416,11 +416,13 @@ void optee_enable_shm_cache(struct optee + } + + /** +- * optee_disable_shm_cache() - Disables caching of some shared memory allocation +- * in OP-TEE ++ * __optee_disable_shm_cache() - Disables caching of some shared memory ++ * allocation in OP-TEE + * @optee: main service struct ++ * @is_mapped: true if the cached shared memory addresses were mapped by this ++ * kernel, are safe to dereference, and should be freed + */ +-void optee_disable_shm_cache(struct optee *optee) ++static void __optee_disable_shm_cache(struct optee *optee, bool is_mapped) + { + struct optee_call_waiter w; + +@@ -439,6 +441,13 @@ void optee_disable_shm_cache(struct opte + if (res.result.status == OPTEE_SMC_RETURN_OK) { + struct tee_shm *shm; + ++ /* ++ * Shared memory references that were not mapped by ++ * this kernel must be ignored to prevent a crash. ++ */ ++ if (!is_mapped) ++ continue; ++ + shm = reg_pair_to_ptr(res.result.shm_upper32, + res.result.shm_lower32); + tee_shm_free(shm); +@@ -449,6 +458,27 @@ void optee_disable_shm_cache(struct opte + optee_cq_wait_final(&optee->call_queue, &w); + } + ++/** ++ * optee_disable_shm_cache() - Disables caching of mapped shared memory ++ * allocations in OP-TEE ++ * @optee: main service struct ++ */ ++void optee_disable_shm_cache(struct optee *optee) ++{ ++ return __optee_disable_shm_cache(optee, true); ++} ++ ++/** ++ * optee_disable_unmapped_shm_cache() - Disables caching of shared memory ++ * allocations in OP-TEE which are not ++ * currently mapped ++ * @optee: main service struct ++ */ ++void optee_disable_unmapped_shm_cache(struct optee *optee) ++{ ++ return __optee_disable_shm_cache(optee, false); ++} ++ + #define PAGELIST_ENTRIES_PER_PAGE \ + ((OPTEE_MSG_NONCONTIG_PAGE_SIZE / sizeof(u64)) - 1) + +--- a/drivers/tee/optee/core.c ++++ b/drivers/tee/optee/core.c +@@ -686,6 +686,15 @@ static int optee_probe(struct platform_d + optee->memremaped_shm = memremaped_shm; + optee->pool = pool; + ++ /* ++ * Ensure that there are no pre-existing shm objects before enabling ++ * the shm cache so that there's no chance of receiving an invalid ++ * address during shutdown. This could occur, for example, if we're ++ * kexec booting from an older kernel that did not properly cleanup the ++ * shm cache. ++ */ ++ optee_disable_unmapped_shm_cache(optee); ++ + optee_enable_shm_cache(optee); + + if (optee->sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM) +--- a/drivers/tee/optee/optee_private.h ++++ b/drivers/tee/optee/optee_private.h +@@ -159,6 +159,7 @@ int optee_cancel_req(struct tee_context + + void optee_enable_shm_cache(struct optee *optee); + void optee_disable_shm_cache(struct optee *optee); ++void optee_disable_unmapped_shm_cache(struct optee *optee); + + int optee_shm_register(struct tee_context *ctx, struct tee_shm *shm, + struct page **pages, size_t num_pages, diff --git a/queue-5.13/optee-fix-memory-leak-when-failing-to-register-shm-pages.patch b/queue-5.13/optee-fix-memory-leak-when-failing-to-register-shm-pages.patch new file mode 100644 index 00000000000..ac1ca2c5e90 --- /dev/null +++ b/queue-5.13/optee-fix-memory-leak-when-failing-to-register-shm-pages.patch @@ -0,0 +1,54 @@ +From ec185dd3ab257dc2a60953fdf1b6622f524cc5b7 Mon Sep 17 00:00:00 2001 +From: Tyler Hicks +Date: Mon, 14 Jun 2021 17:33:10 -0500 +Subject: optee: Fix memory leak when failing to register shm pages + +From: Tyler Hicks + +commit ec185dd3ab257dc2a60953fdf1b6622f524cc5b7 upstream. + +Free the previously allocated pages when we encounter an error condition +while attempting to register the pages with the secure world. + +Fixes: a249dd200d03 ("tee: optee: Fix dynamic shm pool allocations") +Fixes: 5a769f6ff439 ("optee: Fix multi page dynamic shm pool alloc") +Cc: stable@vger.kernel.org +Signed-off-by: Tyler Hicks +Reviewed-by: Jens Wiklander +Reviewed-by: Sumit Garg +Signed-off-by: Jens Wiklander +Signed-off-by: Greg Kroah-Hartman +--- + drivers/tee/optee/shm_pool.c | 12 ++++++++++-- + 1 file changed, 10 insertions(+), 2 deletions(-) + +--- a/drivers/tee/optee/shm_pool.c ++++ b/drivers/tee/optee/shm_pool.c +@@ -36,8 +36,10 @@ static int pool_op_alloc(struct tee_shm_ + struct page **pages; + + pages = kcalloc(nr_pages, sizeof(pages), GFP_KERNEL); +- if (!pages) +- return -ENOMEM; ++ if (!pages) { ++ rc = -ENOMEM; ++ goto err; ++ } + + for (i = 0; i < nr_pages; i++) { + pages[i] = page; +@@ -48,8 +50,14 @@ static int pool_op_alloc(struct tee_shm_ + rc = optee_shm_register(shm->ctx, shm, pages, nr_pages, + (unsigned long)shm->kaddr); + kfree(pages); ++ if (rc) ++ goto err; + } + ++ return 0; ++ ++err: ++ __free_pages(page, order); + return rc; + } + diff --git a/queue-5.13/optee-fix-tee-out-of-memory-failure-seen-during-kexec-reboot.patch b/queue-5.13/optee-fix-tee-out-of-memory-failure-seen-during-kexec-reboot.patch new file mode 100644 index 00000000000..29758d6a2d4 --- /dev/null +++ b/queue-5.13/optee-fix-tee-out-of-memory-failure-seen-during-kexec-reboot.patch @@ -0,0 +1,77 @@ +From f25889f93184db8b07a543cc2bbbb9a8fcaf4333 Mon Sep 17 00:00:00 2001 +From: Allen Pais +Date: Mon, 14 Jun 2021 17:33:12 -0500 +Subject: optee: fix tee out of memory failure seen during kexec reboot + +From: Allen Pais + +commit f25889f93184db8b07a543cc2bbbb9a8fcaf4333 upstream. + +The following out of memory errors are seen on kexec reboot +from the optee core. + +[ 0.368428] tee_bnxt_fw optee-clnt0: tee_shm_alloc failed +[ 0.368461] tee_bnxt_fw: probe of optee-clnt0 failed with error -22 + +tee_shm_release() is not invoked on dma shm buffer. + +Implement .shutdown() method to handle the release of the buffers +correctly. + +More info: +https://github.com/OP-TEE/optee_os/issues/3637 + +Cc: stable@vger.kernel.org +Signed-off-by: Allen Pais +Reviewed-by: Tyler Hicks +Reviewed-by: Jens Wiklander +Reviewed-by: Sumit Garg +Signed-off-by: Jens Wiklander +Signed-off-by: Greg Kroah-Hartman +--- + drivers/tee/optee/core.c | 20 ++++++++++++++++++++ + 1 file changed, 20 insertions(+) + +--- a/drivers/tee/optee/core.c ++++ b/drivers/tee/optee/core.c +@@ -574,6 +574,13 @@ static optee_invoke_fn *get_invoke_func( + return ERR_PTR(-EINVAL); + } + ++/* optee_remove - Device Removal Routine ++ * @pdev: platform device information struct ++ * ++ * optee_remove is called by platform subsystem to alert the driver ++ * that it should release the device ++ */ ++ + static int optee_remove(struct platform_device *pdev) + { + struct optee *optee = platform_get_drvdata(pdev); +@@ -604,6 +611,18 @@ static int optee_remove(struct platform_ + return 0; + } + ++/* optee_shutdown - Device Removal Routine ++ * @pdev: platform device information struct ++ * ++ * platform_shutdown is called by the platform subsystem to alert ++ * the driver that a shutdown, reboot, or kexec is happening and ++ * device must be disabled. ++ */ ++static void optee_shutdown(struct platform_device *pdev) ++{ ++ optee_disable_shm_cache(platform_get_drvdata(pdev)); ++} ++ + static int optee_probe(struct platform_device *pdev) + { + optee_invoke_fn *invoke_fn; +@@ -749,6 +768,7 @@ MODULE_DEVICE_TABLE(of, optee_dt_match); + static struct platform_driver optee_driver = { + .probe = optee_probe, + .remove = optee_remove, ++ .shutdown = optee_shutdown, + .driver = { + .name = "optee", + .of_match_table = optee_dt_match, diff --git a/queue-5.13/optee-refuse-to-load-the-driver-under-the-kdump-kernel.patch b/queue-5.13/optee-refuse-to-load-the-driver-under-the-kdump-kernel.patch new file mode 100644 index 00000000000..2f117e8ce8f --- /dev/null +++ b/queue-5.13/optee-refuse-to-load-the-driver-under-the-kdump-kernel.patch @@ -0,0 +1,87 @@ +From adf752af454e91e123e85e3784972d166837af73 Mon Sep 17 00:00:00 2001 +From: Tyler Hicks +Date: Mon, 14 Jun 2021 17:33:11 -0500 +Subject: optee: Refuse to load the driver under the kdump kernel + +From: Tyler Hicks + +commit adf752af454e91e123e85e3784972d166837af73 upstream. + +Fix a hung task issue, seen when booting the kdump kernel, that is +caused by all of the secure world threads being in a permanent suspended +state: + + INFO: task swapper/0:1 blocked for more than 120 seconds. + Not tainted 5.4.83 #1 + "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. + swapper/0 D 0 1 0 0x00000028 + Call trace: + __switch_to+0xc8/0x118 + __schedule+0x2e0/0x700 + schedule+0x38/0xb8 + schedule_timeout+0x258/0x388 + wait_for_completion+0x16c/0x4b8 + optee_cq_wait_for_completion+0x28/0xa8 + optee_disable_shm_cache+0xb8/0xf8 + optee_probe+0x560/0x61c + platform_drv_probe+0x58/0xa8 + really_probe+0xe0/0x338 + driver_probe_device+0x5c/0xf0 + device_driver_attach+0x74/0x80 + __driver_attach+0x64/0xe0 + bus_for_each_dev+0x84/0xd8 + driver_attach+0x30/0x40 + bus_add_driver+0x188/0x1e8 + driver_register+0x64/0x110 + __platform_driver_register+0x54/0x60 + optee_driver_init+0x20/0x28 + do_one_initcall+0x54/0x24c + kernel_init_freeable+0x1e8/0x2c0 + kernel_init+0x18/0x118 + ret_from_fork+0x10/0x18 + +The invoke_fn hook returned OPTEE_SMC_RETURN_ETHREAD_LIMIT, indicating +that the secure world threads were all in a suspended state at the time +of the kernel crash. This intermittently prevented the kdump kernel from +booting, resulting in a failure to collect the kernel dump. + +Make kernel dump collection more reliable on systems utilizing OP-TEE by +refusing to load the driver under the kdump kernel. + +Cc: stable@vger.kernel.org +Signed-off-by: Tyler Hicks +Reviewed-by: Jens Wiklander +Reviewed-by: Sumit Garg +Signed-off-by: Jens Wiklander +Signed-off-by: Greg Kroah-Hartman +--- + drivers/tee/optee/core.c | 11 +++++++++++ + 1 file changed, 11 insertions(+) + +--- a/drivers/tee/optee/core.c ++++ b/drivers/tee/optee/core.c +@@ -6,6 +6,7 @@ + #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + + #include ++#include + #include + #include + #include +@@ -613,6 +614,16 @@ static int optee_probe(struct platform_d + u32 sec_caps; + int rc; + ++ /* ++ * The kernel may have crashed at the same time that all available ++ * secure world threads were suspended and we cannot reschedule the ++ * suspended threads without access to the crashed kernel's wait_queue. ++ * Therefore, we cannot reliably initialize the OP-TEE driver in the ++ * kdump kernel. ++ */ ++ if (is_kdump_kernel()) ++ return -ENODEV; ++ + invoke_fn = get_invoke_func(&pdev->dev); + if (IS_ERR(invoke_fn)) + return PTR_ERR(invoke_fn); diff --git a/queue-5.13/scripts-tracing-fix-the-bug-that-can-t-parse-raw_trace_func.patch b/queue-5.13/scripts-tracing-fix-the-bug-that-can-t-parse-raw_trace_func.patch new file mode 100644 index 00000000000..c82bd7a4a37 --- /dev/null +++ b/queue-5.13/scripts-tracing-fix-the-bug-that-can-t-parse-raw_trace_func.patch @@ -0,0 +1,66 @@ +From 1c0cec64a7cc545eb49f374a43e9f7190a14defa Mon Sep 17 00:00:00 2001 +From: Hui Su +Date: Fri, 11 Jun 2021 10:21:07 +0800 +Subject: scripts/tracing: fix the bug that can't parse raw_trace_func + +From: Hui Su + +commit 1c0cec64a7cc545eb49f374a43e9f7190a14defa upstream. + +Since commit 77271ce4b2c0 ("tracing: Add irq, preempt-count and need resched info +to default trace output"), the default trace output format has been changed to: + -0 [009] d.h. 22420.068695: _raw_spin_lock_irqsave <-hrtimer_interrupt + -0 [000] ..s. 22420.068695: _nohz_idle_balance <-run_rebalance_domains + -0 [011] d.h. 22420.068695: account_process_tick <-update_process_times + +origin trace output format:(before v3.2.0) + # tracer: nop + # + # TASK-PID CPU# TIMESTAMP FUNCTION + # | | | | | + migration/0-6 [000] 50.025810: rcu_note_context_switch <-__schedule + migration/0-6 [000] 50.025812: trace_rcu_utilization <-rcu_note_context_switch + migration/0-6 [000] 50.025813: rcu_sched_qs <-rcu_note_context_switch + migration/0-6 [000] 50.025815: rcu_preempt_qs <-rcu_note_context_switch + migration/0-6 [000] 50.025817: trace_rcu_utilization <-rcu_note_context_switch + migration/0-6 [000] 50.025818: debug_lockdep_rcu_enabled <-__schedule + migration/0-6 [000] 50.025820: debug_lockdep_rcu_enabled <-__schedule + +The draw_functrace.py(introduced in v2.6.28) can't parse the new version format trace_func, +So we need modify draw_functrace.py to adapt the new version trace output format. + +Link: https://lkml.kernel.org/r/20210611022107.608787-1-suhui@zeku.com + +Cc: stable@vger.kernel.org +Fixes: 77271ce4b2c0 tracing: Add irq, preempt-count and need resched info to default trace output +Signed-off-by: Hui Su +Signed-off-by: Steven Rostedt (VMware) +Signed-off-by: Greg Kroah-Hartman +--- + scripts/tracing/draw_functrace.py | 6 +++--- + 1 file changed, 3 insertions(+), 3 deletions(-) + +--- a/scripts/tracing/draw_functrace.py ++++ b/scripts/tracing/draw_functrace.py +@@ -17,7 +17,7 @@ Usage: + $ cat /sys/kernel/debug/tracing/trace_pipe > ~/raw_trace_func + Wait some times but not too much, the script is a bit slow. + Break the pipe (Ctrl + Z) +- $ scripts/draw_functrace.py < raw_trace_func > draw_functrace ++ $ scripts/tracing/draw_functrace.py < ~/raw_trace_func > draw_functrace + Then you have your drawn trace in draw_functrace + """ + +@@ -103,10 +103,10 @@ def parseLine(line): + line = line.strip() + if line.startswith("#"): + raise CommentLineException +- m = re.match("[^]]+?\\] +([0-9.]+): (\\w+) <-(\\w+)", line) ++ m = re.match("[^]]+?\\] +([a-z.]+) +([0-9.]+): (\\w+) <-(\\w+)", line) + if m is None: + raise BrokenLineException +- return (m.group(1), m.group(2), m.group(3)) ++ return (m.group(2), m.group(3), m.group(4)) + + + def main(): diff --git a/queue-5.13/series b/queue-5.13/series index cd9f3e4a6fa..f4129115081 100644 --- a/queue-5.13/series +++ b/queue-5.13/series @@ -97,3 +97,19 @@ usb-gadget-f_hid-idle-uses-the-highest-byte-for-duration.patch usb-host-ohci-at91-suspend-resume-ports-after-before-ohci-accesses.patch usb-typec-tcpm-keep-other-events-when-receiving-frs-and-sourcing_vbus-events.patch usb-otg-fsm-fix-hrtimer-list-corruption.patch +clk-fix-leak-on-devm_clk_bulk_get_all-unwind.patch +scripts-tracing-fix-the-bug-that-can-t-parse-raw_trace_func.patch +tracing-histogram-give-calculation-hist_fields-a-size.patch +tracing-reject-string-operand-in-the-histogram-expression.patch +tracing-fix-null-pointer-dereference-in-start_creating.patch +tracepoint-static-call-compare-data-on-transition-from-2-1-callees.patch +tracepoint-fix-static-call-function-vs-data-state-mismatch.patch +tracepoint-use-rcu-get-state-and-cond-sync-for-static-call-updates.patch +arm64-stacktrace-avoid-tracing-arch_stack_walk.patch +optee-clear-stale-cache-entries-during-initialization.patch +tee-add-tee_shm_alloc_kernel_buf.patch +tee-correct-inappropriate-usage-of-tee_shm_dma_buf-flag.patch +optee-fix-memory-leak-when-failing-to-register-shm-pages.patch +optee-refuse-to-load-the-driver-under-the-kdump-kernel.patch +optee-fix-tee-out-of-memory-failure-seen-during-kexec-reboot.patch +tpm_ftpm_tee-free-and-unregister-tee-shared-memory-during-kexec.patch diff --git a/queue-5.13/tee-add-tee_shm_alloc_kernel_buf.patch b/queue-5.13/tee-add-tee_shm_alloc_kernel_buf.patch new file mode 100644 index 00000000000..e10cf7cdaf2 --- /dev/null +++ b/queue-5.13/tee-add-tee_shm_alloc_kernel_buf.patch @@ -0,0 +1,60 @@ +From dc7019b7d0e188d4093b34bd0747ed0d668c63bf Mon Sep 17 00:00:00 2001 +From: Jens Wiklander +Date: Mon, 14 Jun 2021 17:33:14 -0500 +Subject: tee: add tee_shm_alloc_kernel_buf() + +From: Jens Wiklander + +commit dc7019b7d0e188d4093b34bd0747ed0d668c63bf upstream. + +Adds a new function tee_shm_alloc_kernel_buf() to allocate shared memory +from a kernel driver. This function can later be made more lightweight +by unnecessary dma-buf export. + +Cc: stable@vger.kernel.org +Reviewed-by: Tyler Hicks +Reviewed-by: Sumit Garg +Signed-off-by: Jens Wiklander +Signed-off-by: Greg Kroah-Hartman +--- + drivers/tee/tee_shm.c | 18 ++++++++++++++++++ + include/linux/tee_drv.h | 1 + + 2 files changed, 19 insertions(+) + +--- a/drivers/tee/tee_shm.c ++++ b/drivers/tee/tee_shm.c +@@ -193,6 +193,24 @@ err_dev_put: + } + EXPORT_SYMBOL_GPL(tee_shm_alloc); + ++/** ++ * tee_shm_alloc_kernel_buf() - Allocate shared memory for kernel buffer ++ * @ctx: Context that allocates the shared memory ++ * @size: Requested size of shared memory ++ * ++ * The returned memory registered in secure world and is suitable to be ++ * passed as a memory buffer in parameter argument to ++ * tee_client_invoke_func(). The memory allocated is later freed with a ++ * call to tee_shm_free(). ++ * ++ * @returns a pointer to 'struct tee_shm' ++ */ ++struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size) ++{ ++ return tee_shm_alloc(ctx, size, TEE_SHM_MAPPED | TEE_SHM_DMA_BUF); ++} ++EXPORT_SYMBOL_GPL(tee_shm_alloc_kernel_buf); ++ + struct tee_shm *tee_shm_register(struct tee_context *ctx, unsigned long addr, + size_t length, u32 flags) + { +--- a/include/linux/tee_drv.h ++++ b/include/linux/tee_drv.h +@@ -332,6 +332,7 @@ void *tee_get_drvdata(struct tee_device + * @returns a pointer to 'struct tee_shm' + */ + struct tee_shm *tee_shm_alloc(struct tee_context *ctx, size_t size, u32 flags); ++struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size); + + /** + * tee_shm_register() - Register shared memory buffer diff --git a/queue-5.13/tee-correct-inappropriate-usage-of-tee_shm_dma_buf-flag.patch b/queue-5.13/tee-correct-inappropriate-usage-of-tee_shm_dma_buf-flag.patch new file mode 100644 index 00000000000..5e337cced83 --- /dev/null +++ b/queue-5.13/tee-correct-inappropriate-usage-of-tee_shm_dma_buf-flag.patch @@ -0,0 +1,133 @@ +From 376e4199e327a5cf29b8ec8fb0f64f3d8b429819 Mon Sep 17 00:00:00 2001 +From: Sumit Garg +Date: Mon, 14 Jun 2021 17:33:15 -0500 +Subject: tee: Correct inappropriate usage of TEE_SHM_DMA_BUF flag + +From: Sumit Garg + +commit 376e4199e327a5cf29b8ec8fb0f64f3d8b429819 upstream. + +Currently TEE_SHM_DMA_BUF flag has been inappropriately used to not +register shared memory allocated for private usage by underlying TEE +driver: OP-TEE in this case. So rather add a new flag as TEE_SHM_PRIV +that can be utilized by underlying TEE drivers for private allocation +and usage of shared memory. + +With this corrected, allow tee_shm_alloc_kernel_buf() to allocate a +shared memory region without the backing of dma-buf. + +Cc: stable@vger.kernel.org +Signed-off-by: Sumit Garg +Co-developed-by: Tyler Hicks +Signed-off-by: Tyler Hicks +Reviewed-by: Jens Wiklander +Reviewed-by: Sumit Garg +Signed-off-by: Jens Wiklander +Signed-off-by: Greg Kroah-Hartman +--- + drivers/tee/optee/call.c | 2 +- + drivers/tee/optee/core.c | 3 ++- + drivers/tee/optee/rpc.c | 5 +++-- + drivers/tee/optee/shm_pool.c | 8 ++++++-- + drivers/tee/tee_shm.c | 4 ++-- + include/linux/tee_drv.h | 1 + + 6 files changed, 15 insertions(+), 8 deletions(-) + +--- a/drivers/tee/optee/call.c ++++ b/drivers/tee/optee/call.c +@@ -184,7 +184,7 @@ static struct tee_shm *get_msg_arg(struc + struct optee_msg_arg *ma; + + shm = tee_shm_alloc(ctx, OPTEE_MSG_GET_ARG_SIZE(num_params), +- TEE_SHM_MAPPED); ++ TEE_SHM_MAPPED | TEE_SHM_PRIV); + if (IS_ERR(shm)) + return shm; + +--- a/drivers/tee/optee/core.c ++++ b/drivers/tee/optee/core.c +@@ -277,7 +277,8 @@ static void optee_release(struct tee_con + if (!ctxdata) + return; + +- shm = tee_shm_alloc(ctx, sizeof(struct optee_msg_arg), TEE_SHM_MAPPED); ++ shm = tee_shm_alloc(ctx, sizeof(struct optee_msg_arg), ++ TEE_SHM_MAPPED | TEE_SHM_PRIV); + if (!IS_ERR(shm)) { + arg = tee_shm_get_va(shm, 0); + /* +--- a/drivers/tee/optee/rpc.c ++++ b/drivers/tee/optee/rpc.c +@@ -314,7 +314,7 @@ static void handle_rpc_func_cmd_shm_allo + shm = cmd_alloc_suppl(ctx, sz); + break; + case OPTEE_RPC_SHM_TYPE_KERNEL: +- shm = tee_shm_alloc(ctx, sz, TEE_SHM_MAPPED); ++ shm = tee_shm_alloc(ctx, sz, TEE_SHM_MAPPED | TEE_SHM_PRIV); + break; + default: + arg->ret = TEEC_ERROR_BAD_PARAMETERS; +@@ -502,7 +502,8 @@ void optee_handle_rpc(struct tee_context + + switch (OPTEE_SMC_RETURN_GET_RPC_FUNC(param->a0)) { + case OPTEE_SMC_RPC_FUNC_ALLOC: +- shm = tee_shm_alloc(ctx, param->a1, TEE_SHM_MAPPED); ++ shm = tee_shm_alloc(ctx, param->a1, ++ TEE_SHM_MAPPED | TEE_SHM_PRIV); + if (!IS_ERR(shm) && !tee_shm_get_pa(shm, 0, &pa)) { + reg_pair_from_64(¶m->a1, ¶m->a2, pa); + reg_pair_from_64(¶m->a4, ¶m->a5, +--- a/drivers/tee/optee/shm_pool.c ++++ b/drivers/tee/optee/shm_pool.c +@@ -27,7 +27,11 @@ static int pool_op_alloc(struct tee_shm_ + shm->paddr = page_to_phys(page); + shm->size = PAGE_SIZE << order; + +- if (shm->flags & TEE_SHM_DMA_BUF) { ++ /* ++ * Shared memory private to the OP-TEE driver doesn't need ++ * to be registered with OP-TEE. ++ */ ++ if (!(shm->flags & TEE_SHM_PRIV)) { + unsigned int nr_pages = 1 << order, i; + struct page **pages; + +@@ -52,7 +56,7 @@ static int pool_op_alloc(struct tee_shm_ + static void pool_op_free(struct tee_shm_pool_mgr *poolm, + struct tee_shm *shm) + { +- if (shm->flags & TEE_SHM_DMA_BUF) ++ if (!(shm->flags & TEE_SHM_PRIV)) + optee_shm_unregister(shm->ctx, shm); + + free_pages((unsigned long)shm->kaddr, get_order(shm->size)); +--- a/drivers/tee/tee_shm.c ++++ b/drivers/tee/tee_shm.c +@@ -117,7 +117,7 @@ struct tee_shm *tee_shm_alloc(struct tee + return ERR_PTR(-EINVAL); + } + +- if ((flags & ~(TEE_SHM_MAPPED | TEE_SHM_DMA_BUF))) { ++ if ((flags & ~(TEE_SHM_MAPPED | TEE_SHM_DMA_BUF | TEE_SHM_PRIV))) { + dev_err(teedev->dev.parent, "invalid shm flags 0x%x", flags); + return ERR_PTR(-EINVAL); + } +@@ -207,7 +207,7 @@ EXPORT_SYMBOL_GPL(tee_shm_alloc); + */ + struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size) + { +- return tee_shm_alloc(ctx, size, TEE_SHM_MAPPED | TEE_SHM_DMA_BUF); ++ return tee_shm_alloc(ctx, size, TEE_SHM_MAPPED); + } + EXPORT_SYMBOL_GPL(tee_shm_alloc_kernel_buf); + +--- a/include/linux/tee_drv.h ++++ b/include/linux/tee_drv.h +@@ -27,6 +27,7 @@ + #define TEE_SHM_USER_MAPPED BIT(4) /* Memory mapped in user space */ + #define TEE_SHM_POOL BIT(5) /* Memory allocated from pool */ + #define TEE_SHM_KERNEL_MAPPED BIT(6) /* Memory mapped in kernel space */ ++#define TEE_SHM_PRIV BIT(7) /* Memory private to TEE driver */ + + struct device; + struct tee_device; diff --git a/queue-5.13/tpm_ftpm_tee-free-and-unregister-tee-shared-memory-during-kexec.patch b/queue-5.13/tpm_ftpm_tee-free-and-unregister-tee-shared-memory-during-kexec.patch new file mode 100644 index 00000000000..1e965e0accf --- /dev/null +++ b/queue-5.13/tpm_ftpm_tee-free-and-unregister-tee-shared-memory-during-kexec.patch @@ -0,0 +1,53 @@ +From dfb703ad2a8d366b829818a558337be779746575 Mon Sep 17 00:00:00 2001 +From: Tyler Hicks +Date: Mon, 14 Jun 2021 17:33:16 -0500 +Subject: tpm_ftpm_tee: Free and unregister TEE shared memory during kexec + +From: Tyler Hicks + +commit dfb703ad2a8d366b829818a558337be779746575 upstream. + +dma-buf backed shared memory cannot be reliably freed and unregistered +during a kexec operation even when tee_shm_free() is called on the shm +from a .shutdown hook. The problem occurs because dma_buf_put() calls +fput() which then uses task_work_add(), with the TWA_RESUME parameter, +to queue tee_shm_release() to be called before the current task returns +to user mode. However, the current task never returns to user mode +before the kexec completes so the memory is never freed nor +unregistered. + +Use tee_shm_alloc_kernel_buf() to avoid dma-buf backed shared memory +allocation so that tee_shm_free() can directly call tee_shm_release(). +This will ensure that the shm can be freed and unregistered during a +kexec operation. + +Fixes: 09e574831b27 ("tpm/tpm_ftpm_tee: A driver for firmware TPM running inside TEE") +Fixes: 1760eb689ed6 ("tpm/tpm_ftpm_tee: add shutdown call back") +Cc: stable@vger.kernel.org +Signed-off-by: Tyler Hicks +Reviewed-by: Sumit Garg +Acked-by: Jarkko Sakkinen +Signed-off-by: Jens Wiklander +Signed-off-by: Greg Kroah-Hartman +--- + drivers/char/tpm/tpm_ftpm_tee.c | 8 ++++---- + 1 file changed, 4 insertions(+), 4 deletions(-) + +--- a/drivers/char/tpm/tpm_ftpm_tee.c ++++ b/drivers/char/tpm/tpm_ftpm_tee.c +@@ -254,11 +254,11 @@ static int ftpm_tee_probe(struct device + pvt_data->session = sess_arg.session; + + /* Allocate dynamic shared memory with fTPM TA */ +- pvt_data->shm = tee_shm_alloc(pvt_data->ctx, +- MAX_COMMAND_SIZE + MAX_RESPONSE_SIZE, +- TEE_SHM_MAPPED | TEE_SHM_DMA_BUF); ++ pvt_data->shm = tee_shm_alloc_kernel_buf(pvt_data->ctx, ++ MAX_COMMAND_SIZE + ++ MAX_RESPONSE_SIZE); + if (IS_ERR(pvt_data->shm)) { +- dev_err(dev, "%s: tee_shm_alloc failed\n", __func__); ++ dev_err(dev, "%s: tee_shm_alloc_kernel_buf failed\n", __func__); + rc = -ENOMEM; + goto out_shm_alloc; + } diff --git a/queue-5.13/tracepoint-fix-static-call-function-vs-data-state-mismatch.patch b/queue-5.13/tracepoint-fix-static-call-function-vs-data-state-mismatch.patch new file mode 100644 index 00000000000..cc5e9d765e5 --- /dev/null +++ b/queue-5.13/tracepoint-fix-static-call-function-vs-data-state-mismatch.patch @@ -0,0 +1,228 @@ +From 231264d6927f6740af36855a622d0e240be9d94c Mon Sep 17 00:00:00 2001 +From: Mathieu Desnoyers +Date: Thu, 5 Aug 2021 09:27:16 -0400 +Subject: tracepoint: Fix static call function vs data state mismatch + +From: Mathieu Desnoyers + +commit 231264d6927f6740af36855a622d0e240be9d94c upstream. + +On a 1->0->1 callbacks transition, there is an issue with the new +callback using the old callback's data. + +Considering __DO_TRACE_CALL: + + do { \ + struct tracepoint_func *it_func_ptr; \ + void *__data; \ + it_func_ptr = \ + rcu_dereference_raw((&__tracepoint_##name)->funcs); \ + if (it_func_ptr) { \ + __data = (it_func_ptr)->data; \ + +----> [ delayed here on one CPU (e.g. vcpu preempted by the host) ] + + static_call(tp_func_##name)(__data, args); \ + } \ + } while (0) + +It has loaded the tp->funcs of the old callback, so it will try to use the old +data. This can be fixed by adding a RCU sync anywhere in the 1->0->1 +transition chain. + +On a N->2->1 transition, we need an rcu-sync because you may have a +sequence of 3->2->1 (or 1->2->1) where the element 0 data is unchanged +between 2->1, but was changed from 3->2 (or from 1->2), which may be +observed by the static call. This can be fixed by adding an +unconditional RCU sync in transition 2->1. + +Note, this fixes a correctness issue at the cost of adding a tremendous +performance regression to the disabling of tracepoints. + +Before this commit: + + # trace-cmd start -e all + # time trace-cmd start -p nop + + real 0m0.778s + user 0m0.000s + sys 0m0.061s + +After this commit: + + # trace-cmd start -e all + # time trace-cmd start -p nop + + real 0m10.593s + user 0m0.017s + sys 0m0.259s + +A follow up fix will introduce a more lightweight scheme based on RCU +get_state and cond_sync, that will return the performance back to what it +was. As both this change and the lightweight versions are complex on their +own, for bisecting any issues that this may cause, they are kept as two +separate changes. + +Link: https://lkml.kernel.org/r/20210805132717.23813-3-mathieu.desnoyers@efficios.com +Link: https://lore.kernel.org/io-uring/4ebea8f0-58c9-e571-fd30-0ce4f6f09c70@samba.org/ + +Cc: stable@vger.kernel.org +Cc: Ingo Molnar +Cc: Peter Zijlstra +Cc: Andrew Morton +Cc: "Paul E. McKenney" +Cc: Stefan Metzmacher +Fixes: d25e37d89dd2 ("tracepoint: Optimize using static_call()") +Signed-off-by: Mathieu Desnoyers +Signed-off-by: Steven Rostedt (VMware) +Signed-off-by: Greg Kroah-Hartman +--- + kernel/tracepoint.c | 102 +++++++++++++++++++++++++++++++++++++++++----------- + 1 file changed, 82 insertions(+), 20 deletions(-) + +--- a/kernel/tracepoint.c ++++ b/kernel/tracepoint.c +@@ -15,6 +15,13 @@ + #include + #include + ++enum tp_func_state { ++ TP_FUNC_0, ++ TP_FUNC_1, ++ TP_FUNC_2, ++ TP_FUNC_N, ++}; ++ + extern tracepoint_ptr_t __start___tracepoints_ptrs[]; + extern tracepoint_ptr_t __stop___tracepoints_ptrs[]; + +@@ -246,26 +253,29 @@ static void *func_remove(struct tracepoi + return old; + } + +-static void tracepoint_update_call(struct tracepoint *tp, struct tracepoint_func *tp_funcs, bool sync) ++/* ++ * Count the number of functions (enum tp_func_state) in a tp_funcs array. ++ */ ++static enum tp_func_state nr_func_state(const struct tracepoint_func *tp_funcs) ++{ ++ if (!tp_funcs) ++ return TP_FUNC_0; ++ if (!tp_funcs[1].func) ++ return TP_FUNC_1; ++ if (!tp_funcs[2].func) ++ return TP_FUNC_2; ++ return TP_FUNC_N; /* 3 or more */ ++} ++ ++static void tracepoint_update_call(struct tracepoint *tp, struct tracepoint_func *tp_funcs) + { + void *func = tp->iterator; + + /* Synthetic events do not have static call sites */ + if (!tp->static_call_key) + return; +- +- if (!tp_funcs[1].func) { ++ if (nr_func_state(tp_funcs) == TP_FUNC_1) + func = tp_funcs[0].func; +- /* +- * If going from the iterator back to a single caller, +- * we need to synchronize with __DO_TRACE to make sure +- * that the data passed to the callback is the one that +- * belongs to that callback. +- */ +- if (sync) +- tracepoint_synchronize_unregister(); +- } +- + __static_call_update(tp->static_call_key, tp->static_call_tramp, func); + } + +@@ -299,9 +309,31 @@ static int tracepoint_add_func(struct tr + * a pointer to it. This array is referenced by __DO_TRACE from + * include/linux/tracepoint.h using rcu_dereference_sched(). + */ +- tracepoint_update_call(tp, tp_funcs, false); +- rcu_assign_pointer(tp->funcs, tp_funcs); +- static_key_enable(&tp->key); ++ switch (nr_func_state(tp_funcs)) { ++ case TP_FUNC_1: /* 0->1 */ ++ /* Set static call to first function */ ++ tracepoint_update_call(tp, tp_funcs); ++ /* Both iterator and static call handle NULL tp->funcs */ ++ rcu_assign_pointer(tp->funcs, tp_funcs); ++ static_key_enable(&tp->key); ++ break; ++ case TP_FUNC_2: /* 1->2 */ ++ /* Set iterator static call */ ++ tracepoint_update_call(tp, tp_funcs); ++ /* ++ * Iterator callback installed before updating tp->funcs. ++ * Requires ordering between RCU assign/dereference and ++ * static call update/call. ++ */ ++ rcu_assign_pointer(tp->funcs, tp_funcs); ++ break; ++ case TP_FUNC_N: /* N->N+1 (N>1) */ ++ rcu_assign_pointer(tp->funcs, tp_funcs); ++ break; ++ default: ++ WARN_ON_ONCE(1); ++ break; ++ } + + release_probes(old); + return 0; +@@ -328,17 +360,47 @@ static int tracepoint_remove_func(struct + /* Failed allocating new tp_funcs, replaced func with stub */ + return 0; + +- if (!tp_funcs) { ++ switch (nr_func_state(tp_funcs)) { ++ case TP_FUNC_0: /* 1->0 */ + /* Removed last function */ + if (tp->unregfunc && static_key_enabled(&tp->key)) + tp->unregfunc(); + + static_key_disable(&tp->key); ++ /* Set iterator static call */ ++ tracepoint_update_call(tp, tp_funcs); ++ /* Both iterator and static call handle NULL tp->funcs */ ++ rcu_assign_pointer(tp->funcs, NULL); ++ /* ++ * Make sure new func never uses old data after a 1->0->1 ++ * transition sequence. ++ * Considering that transition 0->1 is the common case ++ * and don't have rcu-sync, issue rcu-sync after ++ * transition 1->0 to break that sequence by waiting for ++ * readers to be quiescent. ++ */ ++ tracepoint_synchronize_unregister(); ++ break; ++ case TP_FUNC_1: /* 2->1 */ + rcu_assign_pointer(tp->funcs, tp_funcs); +- } else { ++ /* ++ * On 2->1 transition, RCU sync is needed before setting ++ * static call to first callback, because the observer ++ * may have loaded any prior tp->funcs after the last one ++ * associated with an rcu-sync. ++ */ ++ tracepoint_synchronize_unregister(); ++ /* Set static call to first function */ ++ tracepoint_update_call(tp, tp_funcs); ++ break; ++ case TP_FUNC_2: /* N->N-1 (N>2) */ ++ fallthrough; ++ case TP_FUNC_N: + rcu_assign_pointer(tp->funcs, tp_funcs); +- tracepoint_update_call(tp, tp_funcs, +- tp_funcs[0].data != old[0].data); ++ break; ++ default: ++ WARN_ON_ONCE(1); ++ break; + } + release_probes(old); + return 0; diff --git a/queue-5.13/tracepoint-static-call-compare-data-on-transition-from-2-1-callees.patch b/queue-5.13/tracepoint-static-call-compare-data-on-transition-from-2-1-callees.patch new file mode 100644 index 00000000000..5e8ae09ca46 --- /dev/null +++ b/queue-5.13/tracepoint-static-call-compare-data-on-transition-from-2-1-callees.patch @@ -0,0 +1,42 @@ +From f7ec4121256393e1d03274acdca73eb18958f27e Mon Sep 17 00:00:00 2001 +From: Mathieu Desnoyers +Date: Thu, 5 Aug 2021 09:27:15 -0400 +Subject: tracepoint: static call: Compare data on transition from 2->1 callees + +From: Mathieu Desnoyers + +commit f7ec4121256393e1d03274acdca73eb18958f27e upstream. + +On transition from 2->1 callees, we should be comparing .data rather +than .func, because the same callback can be registered twice with +different data, and what we care about here is that the data of array +element 0 is unchanged to skip rcu sync. + +Link: https://lkml.kernel.org/r/20210805132717.23813-2-mathieu.desnoyers@efficios.com +Link: https://lore.kernel.org/io-uring/4ebea8f0-58c9-e571-fd30-0ce4f6f09c70@samba.org/ + +Cc: stable@vger.kernel.org +Cc: Ingo Molnar +Cc: Peter Zijlstra +Cc: Andrew Morton +Cc: "Paul E. McKenney" +Cc: Stefan Metzmacher +Fixes: 547305a64632 ("tracepoint: Fix out of sync data passing by static caller") +Signed-off-by: Mathieu Desnoyers +Signed-off-by: Steven Rostedt (VMware) +Signed-off-by: Greg Kroah-Hartman +--- + kernel/tracepoint.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +--- a/kernel/tracepoint.c ++++ b/kernel/tracepoint.c +@@ -338,7 +338,7 @@ static int tracepoint_remove_func(struct + } else { + rcu_assign_pointer(tp->funcs, tp_funcs); + tracepoint_update_call(tp, tp_funcs, +- tp_funcs[0].func != old[0].func); ++ tp_funcs[0].data != old[0].data); + } + release_probes(old); + return 0; diff --git a/queue-5.13/tracepoint-use-rcu-get-state-and-cond-sync-for-static-call-updates.patch b/queue-5.13/tracepoint-use-rcu-get-state-and-cond-sync-for-static-call-updates.patch new file mode 100644 index 00000000000..1bcca8bdab1 --- /dev/null +++ b/queue-5.13/tracepoint-use-rcu-get-state-and-cond-sync-for-static-call-updates.patch @@ -0,0 +1,185 @@ +From 7b40066c97ec66a44e388f82fcf694987451768f Mon Sep 17 00:00:00 2001 +From: Mathieu Desnoyers +Date: Thu, 5 Aug 2021 15:29:54 -0400 +Subject: tracepoint: Use rcu get state and cond sync for static call updates + +From: Mathieu Desnoyers + +commit 7b40066c97ec66a44e388f82fcf694987451768f upstream. + +State transitions from 1->0->1 and N->2->1 callbacks require RCU +synchronization. Rather than performing the RCU synchronization every +time the state change occurs, which is quite slow when many tracepoints +are registered in batch, instead keep a snapshot of the RCU state on the +most recent transitions which belong to a chain, and conditionally wait +for a grace period on the last transition of the chain if one g.p. has +not elapsed since the last snapshot. + +This applies to both RCU and SRCU. + +This brings the performance regression caused by commit 231264d6927f +("Fix: tracepoint: static call function vs data state mismatch") back to +what it was originally. + +Before this commit: + + # trace-cmd start -e all + # time trace-cmd start -p nop + + real 0m10.593s + user 0m0.017s + sys 0m0.259s + +After this commit: + + # trace-cmd start -e all + # time trace-cmd start -p nop + + real 0m0.878s + user 0m0.000s + sys 0m0.103s + +Link: https://lkml.kernel.org/r/20210805192954.30688-1-mathieu.desnoyers@efficios.com +Link: https://lore.kernel.org/io-uring/4ebea8f0-58c9-e571-fd30-0ce4f6f09c70@samba.org/ + +Cc: stable@vger.kernel.org +Cc: Ingo Molnar +Cc: Peter Zijlstra +Cc: Andrew Morton +Cc: "Paul E. McKenney" +Cc: Stefan Metzmacher +Fixes: 231264d6927f ("Fix: tracepoint: static call function vs data state mismatch") +Signed-off-by: Mathieu Desnoyers +Reviewed-by: Paul E. McKenney +Signed-off-by: Steven Rostedt (VMware) +Signed-off-by: Greg Kroah-Hartman +--- + kernel/tracepoint.c | 81 +++++++++++++++++++++++++++++++++++++++++++--------- + 1 file changed, 67 insertions(+), 14 deletions(-) + +--- a/kernel/tracepoint.c ++++ b/kernel/tracepoint.c +@@ -28,6 +28,44 @@ extern tracepoint_ptr_t __stop___tracepo + DEFINE_SRCU(tracepoint_srcu); + EXPORT_SYMBOL_GPL(tracepoint_srcu); + ++enum tp_transition_sync { ++ TP_TRANSITION_SYNC_1_0_1, ++ TP_TRANSITION_SYNC_N_2_1, ++ ++ _NR_TP_TRANSITION_SYNC, ++}; ++ ++struct tp_transition_snapshot { ++ unsigned long rcu; ++ unsigned long srcu; ++ bool ongoing; ++}; ++ ++/* Protected by tracepoints_mutex */ ++static struct tp_transition_snapshot tp_transition_snapshot[_NR_TP_TRANSITION_SYNC]; ++ ++static void tp_rcu_get_state(enum tp_transition_sync sync) ++{ ++ struct tp_transition_snapshot *snapshot = &tp_transition_snapshot[sync]; ++ ++ /* Keep the latest get_state snapshot. */ ++ snapshot->rcu = get_state_synchronize_rcu(); ++ snapshot->srcu = start_poll_synchronize_srcu(&tracepoint_srcu); ++ snapshot->ongoing = true; ++} ++ ++static void tp_rcu_cond_sync(enum tp_transition_sync sync) ++{ ++ struct tp_transition_snapshot *snapshot = &tp_transition_snapshot[sync]; ++ ++ if (!snapshot->ongoing) ++ return; ++ cond_synchronize_rcu(snapshot->rcu); ++ if (!poll_state_synchronize_srcu(&tracepoint_srcu, snapshot->srcu)) ++ synchronize_srcu(&tracepoint_srcu); ++ snapshot->ongoing = false; ++} ++ + /* Set to 1 to enable tracepoint debug output */ + static const int tracepoint_debug; + +@@ -311,6 +349,11 @@ static int tracepoint_add_func(struct tr + */ + switch (nr_func_state(tp_funcs)) { + case TP_FUNC_1: /* 0->1 */ ++ /* ++ * Make sure new static func never uses old data after a ++ * 1->0->1 transition sequence. ++ */ ++ tp_rcu_cond_sync(TP_TRANSITION_SYNC_1_0_1); + /* Set static call to first function */ + tracepoint_update_call(tp, tp_funcs); + /* Both iterator and static call handle NULL tp->funcs */ +@@ -325,10 +368,15 @@ static int tracepoint_add_func(struct tr + * Requires ordering between RCU assign/dereference and + * static call update/call. + */ +- rcu_assign_pointer(tp->funcs, tp_funcs); +- break; ++ fallthrough; + case TP_FUNC_N: /* N->N+1 (N>1) */ + rcu_assign_pointer(tp->funcs, tp_funcs); ++ /* ++ * Make sure static func never uses incorrect data after a ++ * N->...->2->1 (N>1) transition sequence. ++ */ ++ if (tp_funcs[0].data != old[0].data) ++ tp_rcu_get_state(TP_TRANSITION_SYNC_N_2_1); + break; + default: + WARN_ON_ONCE(1); +@@ -372,24 +420,23 @@ static int tracepoint_remove_func(struct + /* Both iterator and static call handle NULL tp->funcs */ + rcu_assign_pointer(tp->funcs, NULL); + /* +- * Make sure new func never uses old data after a 1->0->1 +- * transition sequence. +- * Considering that transition 0->1 is the common case +- * and don't have rcu-sync, issue rcu-sync after +- * transition 1->0 to break that sequence by waiting for +- * readers to be quiescent. ++ * Make sure new static func never uses old data after a ++ * 1->0->1 transition sequence. + */ +- tracepoint_synchronize_unregister(); ++ tp_rcu_get_state(TP_TRANSITION_SYNC_1_0_1); + break; + case TP_FUNC_1: /* 2->1 */ + rcu_assign_pointer(tp->funcs, tp_funcs); + /* +- * On 2->1 transition, RCU sync is needed before setting +- * static call to first callback, because the observer +- * may have loaded any prior tp->funcs after the last one +- * associated with an rcu-sync. ++ * Make sure static func never uses incorrect data after a ++ * N->...->2->1 (N>2) transition sequence. If the first ++ * element's data has changed, then force the synchronization ++ * to prevent current readers that have loaded the old data ++ * from calling the new function. + */ +- tracepoint_synchronize_unregister(); ++ if (tp_funcs[0].data != old[0].data) ++ tp_rcu_get_state(TP_TRANSITION_SYNC_N_2_1); ++ tp_rcu_cond_sync(TP_TRANSITION_SYNC_N_2_1); + /* Set static call to first function */ + tracepoint_update_call(tp, tp_funcs); + break; +@@ -397,6 +444,12 @@ static int tracepoint_remove_func(struct + fallthrough; + case TP_FUNC_N: + rcu_assign_pointer(tp->funcs, tp_funcs); ++ /* ++ * Make sure static func never uses incorrect data after a ++ * N->...->2->1 (N>2) transition sequence. ++ */ ++ if (tp_funcs[0].data != old[0].data) ++ tp_rcu_get_state(TP_TRANSITION_SYNC_N_2_1); + break; + default: + WARN_ON_ONCE(1); diff --git a/queue-5.13/tracing-fix-null-pointer-dereference-in-start_creating.patch b/queue-5.13/tracing-fix-null-pointer-dereference-in-start_creating.patch new file mode 100644 index 00000000000..3c3b729f4bf --- /dev/null +++ b/queue-5.13/tracing-fix-null-pointer-dereference-in-start_creating.patch @@ -0,0 +1,47 @@ +From ff41c28c4b54052942180d8b3f49e75f1445135a Mon Sep 17 00:00:00 2001 +From: Kamal Agrawal +Date: Fri, 30 Jul 2021 18:53:06 +0530 +Subject: tracing: Fix NULL pointer dereference in start_creating + +From: Kamal Agrawal + +commit ff41c28c4b54052942180d8b3f49e75f1445135a upstream. + +The event_trace_add_tracer() can fail. In this case, it leads to a crash +in start_creating with below call stack. Handle the error scenario +properly in trace_array_create_dir. + +Call trace: +down_write+0x7c/0x204 +start_creating.25017+0x6c/0x194 +tracefs_create_file+0xc4/0x2b4 +init_tracer_tracefs+0x5c/0x940 +trace_array_create_dir+0x58/0xb4 +trace_array_create+0x1bc/0x2b8 +trace_array_get_by_name+0xdc/0x18c + +Link: https://lkml.kernel.org/r/1627651386-21315-1-git-send-email-kamaagra@codeaurora.org + +Cc: stable@vger.kernel.org +Fixes: 4114fbfd02f1 ("tracing: Enable creating new instance early boot") +Signed-off-by: Kamal Agrawal +Signed-off-by: Steven Rostedt (VMware) +Signed-off-by: Greg Kroah-Hartman +--- + kernel/trace/trace.c | 4 +++- + 1 file changed, 3 insertions(+), 1 deletion(-) + +--- a/kernel/trace/trace.c ++++ b/kernel/trace/trace.c +@@ -9006,8 +9006,10 @@ static int trace_array_create_dir(struct + return -EINVAL; + + ret = event_trace_add_tracer(tr->dir, tr); +- if (ret) ++ if (ret) { + tracefs_remove(tr->dir); ++ return ret; ++ } + + init_tracer_tracefs(tr, tr->dir); + __update_tracer_options(tr); diff --git a/queue-5.13/tracing-histogram-give-calculation-hist_fields-a-size.patch b/queue-5.13/tracing-histogram-give-calculation-hist_fields-a-size.patch new file mode 100644 index 00000000000..58248fa83bb --- /dev/null +++ b/queue-5.13/tracing-histogram-give-calculation-hist_fields-a-size.patch @@ -0,0 +1,65 @@ +From 2c05caa7ba8803209769b9e4fe02c38d77ae88d0 Mon Sep 17 00:00:00 2001 +From: "Steven Rostedt (VMware)" +Date: Fri, 30 Jul 2021 17:19:51 -0400 +Subject: tracing / histogram: Give calculation hist_fields a size + +From: Steven Rostedt (VMware) + +commit 2c05caa7ba8803209769b9e4fe02c38d77ae88d0 upstream. + +When working on my user space applications, I found a bug in the synthetic +event code where the automated synthetic event field was not matching the +event field calculation it was attached to. Looking deeper into it, it was +because the calculation hist_field was not given a size. + +The synthetic event fields are matched to their hist_fields either by +having the field have an identical string type, or if that does not match, +then the size and signed values are used to match the fields. + +The problem arose when I tried to match a calculation where the fields +were "unsigned int". My tool created a synthetic event of type "u32". But +it failed to match. The string was: + + diff=field1-field2:onmatch(event).trace(synth,$diff) + +Adding debugging into the kernel, I found that the size of "diff" was 0. +And since it was given "unsigned int" as a type, the histogram fallback +code used size and signed. The signed matched, but the size of u32 (4) did +not match zero, and the event failed to be created. + +This can be worse if the field you want to match is not one of the +acceptable fields for a synthetic event. As event fields can have any type +that is supported in Linux, this can cause an issue. For example, if a +type is an enum. Then there's no way to use that with any calculations. + +Have the calculation field simply take on the size of what it is +calculating. + +Link: https://lkml.kernel.org/r/20210730171951.59c7743f@oasis.local.home + +Cc: Tom Zanussi +Cc: Masami Hiramatsu +Cc: Namhyung Kim +Cc: Ingo Molnar +Cc: Andrew Morton +Cc: stable@vger.kernel.org +Fixes: 100719dcef447 ("tracing: Add simple expression support to hist triggers") +Signed-off-by: Steven Rostedt (VMware) +Signed-off-by: Greg Kroah-Hartman +--- + kernel/trace/trace_events_hist.c | 4 ++++ + 1 file changed, 4 insertions(+) + +--- a/kernel/trace/trace_events_hist.c ++++ b/kernel/trace/trace_events_hist.c +@@ -2287,6 +2287,10 @@ static struct hist_field *parse_expr(str + + expr->operands[0] = operand1; + expr->operands[1] = operand2; ++ ++ /* The operand sizes should be the same, so just pick one */ ++ expr->size = operand1->size; ++ + expr->operator = field_op; + expr->name = expr_str(expr, 0); + expr->type = kstrdup(operand1->type, GFP_KERNEL); diff --git a/queue-5.13/tracing-reject-string-operand-in-the-histogram-expression.patch b/queue-5.13/tracing-reject-string-operand-in-the-histogram-expression.patch new file mode 100644 index 00000000000..9a25ca718bb --- /dev/null +++ b/queue-5.13/tracing-reject-string-operand-in-the-histogram-expression.patch @@ -0,0 +1,74 @@ +From a9d10ca4986571bffc19778742d508cc8dd13e02 Mon Sep 17 00:00:00 2001 +From: Masami Hiramatsu +Date: Wed, 28 Jul 2021 07:55:43 +0900 +Subject: tracing: Reject string operand in the histogram expression + +From: Masami Hiramatsu + +commit a9d10ca4986571bffc19778742d508cc8dd13e02 upstream. + +Since the string type can not be the target of the addition / subtraction +operation, it must be rejected. Without this fix, the string type silently +converted to digits. + +Link: https://lkml.kernel.org/r/162742654278.290973.1523000673366456634.stgit@devnote2 + +Cc: stable@vger.kernel.org +Fixes: 100719dcef447 ("tracing: Add simple expression support to hist triggers") +Signed-off-by: Masami Hiramatsu +Signed-off-by: Steven Rostedt (VMware) +Signed-off-by: Greg Kroah-Hartman +--- + kernel/trace/trace_events_hist.c | 20 +++++++++++++++++++- + 1 file changed, 19 insertions(+), 1 deletion(-) + +--- a/kernel/trace/trace_events_hist.c ++++ b/kernel/trace/trace_events_hist.c +@@ -65,7 +65,8 @@ + C(INVALID_SORT_MODIFIER,"Invalid sort modifier"), \ + C(EMPTY_SORT_FIELD, "Empty sort field"), \ + C(TOO_MANY_SORT_FIELDS, "Too many sort fields (Max = 2)"), \ +- C(INVALID_SORT_FIELD, "Sort field must be a key or a val"), ++ C(INVALID_SORT_FIELD, "Sort field must be a key or a val"), \ ++ C(INVALID_STR_OPERAND, "String type can not be an operand in expression"), + + #undef C + #define C(a, b) HIST_ERR_##a +@@ -2156,6 +2157,13 @@ static struct hist_field *parse_unary(st + ret = PTR_ERR(operand1); + goto free; + } ++ if (operand1->flags & HIST_FIELD_FL_STRING) { ++ /* String type can not be the operand of unary operator. */ ++ hist_err(file->tr, HIST_ERR_INVALID_STR_OPERAND, errpos(str)); ++ destroy_hist_field(operand1, 0); ++ ret = -EINVAL; ++ goto free; ++ } + + expr->flags |= operand1->flags & + (HIST_FIELD_FL_TIMESTAMP | HIST_FIELD_FL_TIMESTAMP_USECS); +@@ -2257,6 +2265,11 @@ static struct hist_field *parse_expr(str + operand1 = NULL; + goto free; + } ++ if (operand1->flags & HIST_FIELD_FL_STRING) { ++ hist_err(file->tr, HIST_ERR_INVALID_STR_OPERAND, errpos(operand1_str)); ++ ret = -EINVAL; ++ goto free; ++ } + + /* rest of string could be another expression e.g. b+c in a+b+c */ + operand_flags = 0; +@@ -2266,6 +2279,11 @@ static struct hist_field *parse_expr(str + operand2 = NULL; + goto free; + } ++ if (operand2->flags & HIST_FIELD_FL_STRING) { ++ hist_err(file->tr, HIST_ERR_INVALID_STR_OPERAND, errpos(str)); ++ ret = -EINVAL; ++ goto free; ++ } + + ret = check_expr_operands(file->tr, operand1, operand2); + if (ret) -- 2.47.3