* [PATCH v2] Fix trampoline use of BPF stack for scratch
@ 2025-01-16 23:14 eugene.loh
2025-02-26 16:57 ` Kris Van Hees
0 siblings, 1 reply; 2+ messages in thread
From: eugene.loh @ 2025-01-16 23:14 UTC (permalink / raw)
To: dtrace, dtrace-devel
From: Eugene Loh <eugene.loh@oracle.com>
The trampoline starts by setting %r9 = %fp + -DCTX_SIZE -- that is,
%r9 = %fp - 88. Then, it fills the BPF stack from %fp-88 to %fp with
the various dctx-> pointers. Calls to clauses will reset %fp.
The proc provider's trampoline was using the BPF stack for scratch
space for the exit and signal-handle probes. Specifically, it used
%fp + DT_STK_SPILL(0), which overwrites a dctx-> pointer.
Switch to DT_TRAMP_SP_SLOT(0), which is intended for this purpose.
Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
libdtrace/dt_prov_proc.c | 14 +++++++-------
test/unittest/lquantize/tst.32bit-bug26268136.sh | 1 -
2 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/libdtrace/dt_prov_proc.c b/libdtrace/dt_prov_proc.c
index 0223c5d18..9fc499aeb 100644
--- a/libdtrace/dt_prov_proc.c
+++ b/libdtrace/dt_prov_proc.c
@@ -62,7 +62,7 @@ static const char modname[] = "vmlinux";
* The dependent probe support should include a priority specification to drive
* the order in which dependent probes are added to the underlying probe. This
* is needed to enforce specific probe firing semantics (e.g. proc:::start must
- * always precede [roc:::lwp-start).
+ * always precede proc:::lwp-start).
*/
typedef struct probe_arg {
@@ -350,12 +350,12 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
* else args[0] = 2; // CLD_KILLED
*/
emit(dlp, BPF_MOV_REG(BPF_REG_1, BPF_REG_FP));
- emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, DT_STK_SPILL(0)));
+ emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, DT_TRAMP_SP_SLOT(0)));
emit(dlp, BPF_MOV_IMM(BPF_REG_2, sizeof(int)));
emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_3, BPF_REG_7, DMST_ARG(0)));
emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, ctm.ctm_offset / NBBY));
emit(dlp, BPF_CALL_HELPER(BPF_FUNC_probe_read));
- emit(dlp, BPF_LOAD(BPF_W, BPF_REG_1, BPF_REG_FP, DT_STK_SPILL(0)));
+ emit(dlp, BPF_LOAD(BPF_W, BPF_REG_1, BPF_REG_FP, DT_TRAMP_SP_SLOT(0)));
emit(dlp, BPF_MOV_IMM(BPF_REG_0, 1));
emit(dlp, BPF_MOV_REG(BPF_REG_2, BPF_REG_1));
emit(dlp, BPF_ALU64_IMM(BPF_AND, BPF_REG_2, 0x7f));
@@ -406,17 +406,17 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
emit(dlp, BPF_MOV_REG(BPF_REG_3, BPF_REG_0));
emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, off));
emit(dlp, BPF_MOV_REG(BPF_REG_1, BPF_REG_FP));
- emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, DT_STK_SPILL(0)));
+ emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, DT_TRAMP_SP_SLOT(0)));
emit(dlp, BPF_MOV_IMM(BPF_REG_2, sz));
emit(dlp, BPF_CALL_HELPER(BPF_FUNC_probe_read));
- emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_3, BPF_REG_FP, DT_STK_SPILL(0)));
+ emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_3, BPF_REG_FP, DT_TRAMP_SP_SLOT(0)));
off = dt_cg_ctf_offsetof("struct signal_struct", "group_exit_code", &sz, 0);
emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, off));
emit(dlp, BPF_MOV_REG(BPF_REG_1, BPF_REG_FP));
- emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, DT_STK_SPILL(0)));
+ emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, DT_TRAMP_SP_SLOT(0)));
emit(dlp, BPF_MOV_IMM(BPF_REG_2, sz));
emit(dlp, BPF_CALL_HELPER(BPF_FUNC_probe_read));
- emit(dlp, BPF_LOAD(BPF_W, BPF_REG_0, BPF_REG_FP, DT_STK_SPILL(0)));
+ emit(dlp, BPF_LOAD(BPF_W, BPF_REG_0, BPF_REG_FP, DT_TRAMP_SP_SLOT(0)));
emit(dlp, BPF_BRANCH_IMM(BPF_JEQ, BPF_REG_0, 0, lbl_keep));
emit(dlp, BPF_STORE(BPF_DW, BPF_REG_7, DMST_ARG(0), BPF_REG_0));
diff --git a/test/unittest/lquantize/tst.32bit-bug26268136.sh b/test/unittest/lquantize/tst.32bit-bug26268136.sh
index d5f143f58..a360fd17e 100755
--- a/test/unittest/lquantize/tst.32bit-bug26268136.sh
+++ b/test/unittest/lquantize/tst.32bit-bug26268136.sh
@@ -5,7 +5,6 @@
# Licensed under the Universal Permissive License v 1.0 as shown at
# http://oss.oracle.com/licenses/upl.
#
-# @@xfail: dtv2
if [ $# != 1 ]; then
echo expected one argument: '<'dtrace-path'>'
--
2.43.5
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH v2] Fix trampoline use of BPF stack for scratch
2025-01-16 23:14 [PATCH v2] Fix trampoline use of BPF stack for scratch eugene.loh
@ 2025-02-26 16:57 ` Kris Van Hees
0 siblings, 0 replies; 2+ messages in thread
From: Kris Van Hees @ 2025-02-26 16:57 UTC (permalink / raw)
To: eugene.loh; +Cc: dtrace, dtrace-devel
On Thu, Jan 16, 2025 at 06:14:14PM -0500, eugene.loh@oracle.com wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
>
> The trampoline starts by setting %r9 = %fp + -DCTX_SIZE -- that is,
> %r9 = %fp - 88. Then, it fills the BPF stack from %fp-88 to %fp with
> the various dctx-> pointers. Calls to clauses will reset %fp.
>
> The proc provider's trampoline was using the BPF stack for scratch
> space for the exit and signal-handle probes. Specifically, it used
> %fp + DT_STK_SPILL(0), which overwrites a dctx-> pointer.
>
> Switch to DT_TRAMP_SP_SLOT(0), which is intended for this purpose.
>
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>
> ---
> libdtrace/dt_prov_proc.c | 14 +++++++-------
> test/unittest/lquantize/tst.32bit-bug26268136.sh | 1 -
> 2 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/libdtrace/dt_prov_proc.c b/libdtrace/dt_prov_proc.c
> index 0223c5d18..9fc499aeb 100644
> --- a/libdtrace/dt_prov_proc.c
> +++ b/libdtrace/dt_prov_proc.c
> @@ -62,7 +62,7 @@ static const char modname[] = "vmlinux";
> * The dependent probe support should include a priority specification to drive
> * the order in which dependent probes are added to the underlying probe. This
> * is needed to enforce specific probe firing semantics (e.g. proc:::start must
> - * always precede [roc:::lwp-start).
> + * always precede proc:::lwp-start).
> */
>
> typedef struct probe_arg {
> @@ -350,12 +350,12 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
> * else args[0] = 2; // CLD_KILLED
> */
> emit(dlp, BPF_MOV_REG(BPF_REG_1, BPF_REG_FP));
> - emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, DT_STK_SPILL(0)));
> + emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, DT_TRAMP_SP_SLOT(0)));
> emit(dlp, BPF_MOV_IMM(BPF_REG_2, sizeof(int)));
> emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_3, BPF_REG_7, DMST_ARG(0)));
> emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, ctm.ctm_offset / NBBY));
> emit(dlp, BPF_CALL_HELPER(BPF_FUNC_probe_read));
> - emit(dlp, BPF_LOAD(BPF_W, BPF_REG_1, BPF_REG_FP, DT_STK_SPILL(0)));
> + emit(dlp, BPF_LOAD(BPF_W, BPF_REG_1, BPF_REG_FP, DT_TRAMP_SP_SLOT(0)));
> emit(dlp, BPF_MOV_IMM(BPF_REG_0, 1));
> emit(dlp, BPF_MOV_REG(BPF_REG_2, BPF_REG_1));
> emit(dlp, BPF_ALU64_IMM(BPF_AND, BPF_REG_2, 0x7f));
> @@ -406,17 +406,17 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
> emit(dlp, BPF_MOV_REG(BPF_REG_3, BPF_REG_0));
> emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, off));
> emit(dlp, BPF_MOV_REG(BPF_REG_1, BPF_REG_FP));
> - emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, DT_STK_SPILL(0)));
> + emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, DT_TRAMP_SP_SLOT(0)));
> emit(dlp, BPF_MOV_IMM(BPF_REG_2, sz));
> emit(dlp, BPF_CALL_HELPER(BPF_FUNC_probe_read));
> - emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_3, BPF_REG_FP, DT_STK_SPILL(0)));
> + emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_3, BPF_REG_FP, DT_TRAMP_SP_SLOT(0)));
> off = dt_cg_ctf_offsetof("struct signal_struct", "group_exit_code", &sz, 0);
> emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, off));
> emit(dlp, BPF_MOV_REG(BPF_REG_1, BPF_REG_FP));
> - emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, DT_STK_SPILL(0)));
> + emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, DT_TRAMP_SP_SLOT(0)));
> emit(dlp, BPF_MOV_IMM(BPF_REG_2, sz));
> emit(dlp, BPF_CALL_HELPER(BPF_FUNC_probe_read));
> - emit(dlp, BPF_LOAD(BPF_W, BPF_REG_0, BPF_REG_FP, DT_STK_SPILL(0)));
> + emit(dlp, BPF_LOAD(BPF_W, BPF_REG_0, BPF_REG_FP, DT_TRAMP_SP_SLOT(0)));
> emit(dlp, BPF_BRANCH_IMM(BPF_JEQ, BPF_REG_0, 0, lbl_keep));
> emit(dlp, BPF_STORE(BPF_DW, BPF_REG_7, DMST_ARG(0), BPF_REG_0));
>
> diff --git a/test/unittest/lquantize/tst.32bit-bug26268136.sh b/test/unittest/lquantize/tst.32bit-bug26268136.sh
> index d5f143f58..a360fd17e 100755
> --- a/test/unittest/lquantize/tst.32bit-bug26268136.sh
> +++ b/test/unittest/lquantize/tst.32bit-bug26268136.sh
> @@ -5,7 +5,6 @@
> # Licensed under the Universal Permissive License v 1.0 as shown at
> # http://oss.oracle.com/licenses/upl.
> #
> -# @@xfail: dtv2
>
> if [ $# != 1 ]; then
> echo expected one argument: '<'dtrace-path'>'
> --
> 2.43.5
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-02-26 16:57 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-16 23:14 [PATCH v2] Fix trampoline use of BPF stack for scratch eugene.loh
2025-02-26 16:57 ` Kris Van Hees
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox