[DTrace-devel] [PATCH 2/2 REVIEWED] Add run-time checks on bounds for scalar-array access
Kris Van Hees
kris.van.hees at oracle.com
Fri Feb 24 17:57:50 UTC 2023
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>
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>
---
libdtrace/dt_cg.c | 34 +++++++++++++++++++
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, 101 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 6cf311be..b8ec4e7e 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -3221,6 +3221,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;
@@ -3292,6 +3295,37 @@ 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));
+
+ /* Report out-of-bounds fault on the index. */
+ emit(dlp, BPF_ALU64_IMM(BPF_DIV, dnp->dn_right->dn_reg, elem_size));
+ dt_cg_probe_error(yypcb, DTRACEFLT_BADINDEX, 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..bffa7236
--- /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): index out of bounds ({ptr}) in action #1 at BPF pc NNN
--
2.39.1
More information about the DTrace-devel
mailing list