[DTrace-devel] [PATCH] Add run-time checks on bounds for scalar-array access
eugene.loh at oracle.com
eugene.loh at oracle.com
Wed Feb 8 06:55:29 UTC 2023
From: Eugene Loh <eugene.loh at oracle.com>
If a scalar (nonassociated, linearly indexed) array is accessed with
a constant index, the parser checks that the access is in-bounds.
With other index types, however, the value is not known until runtime.
Since the index value is not being checked at runtime, the BPF verifier
does not allow the code to be loaded, lest an invalid address is
dereferenced.
Add a runtime check when we try to add an offset to a scalar array
address.
Orabug: 35045463
Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
---
libdtrace/dt_cg.c | 39 +++++++++++++++++++
test/unittest/arrays/tst.declared-bounds.d | 1 -
test/unittest/arrays/tst.declared-bounds.r | 2 +-
.../arrays/tst.declared-bounds.runtime_in.d | 31 +++++++++++++++
.../arrays/tst.declared-bounds.runtime_in.r | 1 +
.../arrays/tst.declared-bounds.runtime_out.d | 31 +++++++++++++++
.../arrays/tst.declared-bounds.runtime_out.r | 3 ++
7 files changed, 106 insertions(+), 2 deletions(-)
create mode 100644 test/unittest/arrays/tst.declared-bounds.runtime_in.d
create mode 100644 test/unittest/arrays/tst.declared-bounds.runtime_in.r
create mode 100644 test/unittest/arrays/tst.declared-bounds.runtime_out.d
create mode 100644 test/unittest/arrays/tst.declared-bounds.runtime_out.r
diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index c2eb85c0..50846942 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -3212,6 +3212,9 @@ dt_cg_arithmetic_op(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
int lp_is_ptr = dt_node_is_pointer(dnp->dn_left);
int rp_is_ptr = dt_node_is_pointer(dnp->dn_right);
+ ctf_file_t *ctfp = dnp->dn_left->dn_ctfp;
+ ctf_id_t type = ctf_type_resolve(ctfp, dnp->dn_left->dn_type);
+
if (lp_is_ptr && rp_is_ptr) {
assert(dnp->dn_op == DT_TOK_SUB);
is_ptr_op = 0;
@@ -3283,6 +3286,42 @@ dt_cg_arithmetic_op(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
emitl(dlp, lbl_L5,
BPF_NOP());
+ } else if (op == BPF_ADD
+ && dnp->dn_right->dn_kind != DT_NODE_INT
+ && ctf_type_kind(ctfp, type) == CTF_K_ARRAY) {
+ /*
+ * If the left-hand argument is a CTF_K_ARRAY and we are adding
+ * an offset -- which is exactly what the parser will try to do
+ * if an element of a scalar array is being accessed -- then the
+ * BPF verifier will complain if it knows nothing about the offset.
+ * Meanwhile, if the offset is an DT_NODE_INT, then we have already
+ * checked it is in bounds. So, add a run-time check only when
+ * the right-hand argument is not DT_NODE_INT.
+ */
+ ctf_arinfo_t r;
+ uint_t L1 = dt_irlist_label(dlp);
+ ssize_t elem_size;
+
+ if (ctf_array_info(ctfp, type, &r) != 0) {
+ yypcb->pcb_hdl->dt_ctferr = ctf_errno(ctfp);
+ longjmp(yypcb->pcb_jmpbuf, EDT_CTF);
+ }
+
+ elem_size = ctf_type_size(ctfp, r.ctr_contents);
+
+ emit(dlp, BPF_BRANCH_IMM(BPF_JLT, dnp->dn_right->dn_reg, r.ctr_nelems * elem_size, L1));
+
+ /*
+ * In case of an error, report the index rather than the address, because:
+ * * the address probably means nothing to the user
+ * * we would have to do some trickery to fool the
+ * BPF verifier to let us compute the address.
+ */
+ emit(dlp, BPF_ALU64_IMM(BPF_DIV, dnp->dn_right->dn_reg, elem_size));
+ dt_cg_probe_error(yypcb, DTRACEFLT_BADADDR, DT_ISREG, dnp->dn_right->dn_reg);
+
+ emitl(dlp, L1,
+ BPF_ALU64_REG(op, dnp->dn_left->dn_reg, dnp->dn_right->dn_reg));
} else
emit(dlp, BPF_ALU64_REG(op, dnp->dn_left->dn_reg, dnp->dn_right->dn_reg));
diff --git a/test/unittest/arrays/tst.declared-bounds.d b/test/unittest/arrays/tst.declared-bounds.d
index ff7f3527..b11b5cb7 100644
--- a/test/unittest/arrays/tst.declared-bounds.d
+++ b/test/unittest/arrays/tst.declared-bounds.d
@@ -4,7 +4,6 @@
* Licensed under the Universal Permissive License v 1.0 as shown at
* http://oss.oracle.com/licenses/upl.
*/
-/* @@xfail: dtv2 */
/*
* ASSERTION:
diff --git a/test/unittest/arrays/tst.declared-bounds.r b/test/unittest/arrays/tst.declared-bounds.r
index 11573a18..b604cc0a 100644
--- a/test/unittest/arrays/tst.declared-bounds.r
+++ b/test/unittest/arrays/tst.declared-bounds.r
@@ -1,5 +1,5 @@
FUNCTION:NAME
- :BEGIN 0
+ :BEGIN 0
-- @@stderr --
dtrace: script 'test/unittest/arrays/tst.declared-bounds.d' matched 1 probe
diff --git a/test/unittest/arrays/tst.declared-bounds.runtime_in.d b/test/unittest/arrays/tst.declared-bounds.runtime_in.d
new file mode 100644
index 00000000..8e36e428
--- /dev/null
+++ b/test/unittest/arrays/tst.declared-bounds.runtime_in.d
@@ -0,0 +1,31 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+ * Licensed under the Universal Permissive License v 1.0 as shown at
+ * http://oss.oracle.com/licenses/upl.
+ */
+
+/*
+ * ASSERTION: Within-bounds array assignments work even when index
+ * requires run-time check.
+ *
+ * SECTION: Pointers and Arrays/Array Declarations and Storage
+ */
+
+#pragma D option quiet
+
+int a[5];
+
+BEGIN
+{
+ i = 0;
+ a[i] = 12345678;
+ trace(a[i]);
+ exit(0);
+}
+
+ERROR
+{
+ trace("run-time error");
+ exit(1);
+}
diff --git a/test/unittest/arrays/tst.declared-bounds.runtime_in.r b/test/unittest/arrays/tst.declared-bounds.runtime_in.r
new file mode 100644
index 00000000..97b5955f
--- /dev/null
+++ b/test/unittest/arrays/tst.declared-bounds.runtime_in.r
@@ -0,0 +1 @@
+12345678
diff --git a/test/unittest/arrays/tst.declared-bounds.runtime_out.d b/test/unittest/arrays/tst.declared-bounds.runtime_out.d
new file mode 100644
index 00000000..95397491
--- /dev/null
+++ b/test/unittest/arrays/tst.declared-bounds.runtime_out.d
@@ -0,0 +1,31 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+ * Licensed under the Universal Permissive License v 1.0 as shown at
+ * http://oss.oracle.com/licenses/upl.
+ */
+
+/*
+ * ASSERTION: Out-of-bounds array assignments work even when index
+ * requires run-time check.
+ *
+ * SECTION: Pointers and Arrays/Array Declarations and Storage
+ */
+
+#pragma D option quiet
+
+int a[5];
+
+BEGIN
+{
+ i = 8;
+ a[i] = 12345678;
+ trace(a[i]);
+ exit(1);
+}
+
+ERROR
+{
+ trace("expected run-time error");
+ exit(0);
+}
diff --git a/test/unittest/arrays/tst.declared-bounds.runtime_out.r b/test/unittest/arrays/tst.declared-bounds.runtime_out.r
new file mode 100644
index 00000000..5b4d3eb5
--- /dev/null
+++ b/test/unittest/arrays/tst.declared-bounds.runtime_out.r
@@ -0,0 +1,3 @@
+expected run-time error
+-- @@stderr --
+dtrace: error on enabled probe ID 3 (ID 1: dtrace:::BEGIN): invalid address ({ptr}) in action #1 at BPF pc NNN
--
2.18.4
More information about the DTrace-devel
mailing list