[DTrace-devel] [PATCH 17/20] Allocate extra space in the strtab to help the BPF verifier
Kris Van Hees
kris.van.hees at oracle.com
Tue Jun 1 22:48:20 PDT 2021
Although the internal consistency of the strtab data (generated by
the compiler) guarantees that access to string constants will never
read outside the strtab, the BPF verifier cannot verify that through
static code analysis. By padding the strtab with enough space to
store the largest supported string we guarantee that any access up
to the maximum string size can succeed.
Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
---
libdtrace/dt_bpf.c | 18 ++++++++++++------
.../unittest/actions/freopen/tst.badfreopen.sh | 1 -
test/unittest/operators/tst.ternary.d | 1 -
test/unittest/printf/tst.basics.d | 1 -
test/unittest/printf/tst.precs.d | 1 -
test/unittest/printf/tst.wp.d | 1 -
test/unittest/regression/tst.sigaltstack.d | 1 -
test/unittest/scripting/tst.arg0.d | 1 -
test/unittest/scripting/tst.stringmacro.sh | 1 -
test/unittest/struct/tst.clauselocal.d | 1 -
10 files changed, 12 insertions(+), 15 deletions(-)
diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
index 067bf0d5..7317abfc 100644
--- a/libdtrace/dt_bpf.c
+++ b/libdtrace/dt_bpf.c
@@ -171,7 +171,9 @@ set_task_offsets(dtrace_hdl_t *dtp)
* element (key 0) that contains the entire string table as a
* concatenation of all unique strings (each terminated with a
* NUL byte). The string table size is taken from the DTrace
- * consumer handle (dt_strlen).
+ * consumer handle (dt_strlen), and increased by the maximum
+ * string size to ensure that the BPF verifier can validate all
+ * access requests for dynamic references to string constants.
* - gvars: Global variables map. This is a global map with a singleton
* element (key 0) addressed by variable offset.
* - lvars: Local variables map. This is a per-CPU map with a singleton
@@ -198,7 +200,7 @@ set_task_offsets(dtrace_hdl_t *dtp)
int
dt_bpf_gmap_create(dtrace_hdl_t *dtp)
{
- int gvarsz, lvarsz, tvarc, aggsz;
+ int stabsz, gvarsz, lvarsz, tvarc, aggsz;
int ci_mapfd, st_mapfd;
uint32_t key = 0;
@@ -253,11 +255,15 @@ dt_bpf_gmap_create(dtrace_hdl_t *dtp)
return -1; /* dt_errno is set for us */
/*
- * We may need to create a final copy of the string table because it is
- * possible entries were added after the last clause was compiled.
+ * We need to create the global (consolidated) string table. We store
+ * the actual length (for in-code BPF validation purposes) but augment
+ * it by the maximum string size to determine the size of the BPF map
+ * vaule that is used to store the strtab.
*/
dtp->dt_strlen = dt_strtab_size(dtp->dt_ccstab);
- dtp->dt_strtab = dt_zalloc(dtp, dtp->dt_strlen);
+ stabsz = dtp->dt_strlen +
+ ctf_type_size(DT_STR_CTFP(dtp), DT_STR_TYPE(dtp));
+ dtp->dt_strtab = dt_zalloc(dtp, stabsz);
if (dtp->dt_strtab == NULL)
return dt_set_errno(dtp, EDT_NOMEM);
@@ -265,7 +271,7 @@ dt_bpf_gmap_create(dtrace_hdl_t *dtp)
dtp->dt_strtab);
st_mapfd = create_gmap(dtp, "strtab", BPF_MAP_TYPE_ARRAY,
- sizeof(uint32_t), dtp->dt_strlen, 1);
+ sizeof(uint32_t), stabsz, 1);
if (st_mapfd == -1)
return -1; /* dt_errno is set for us */
diff --git a/test/unittest/actions/freopen/tst.badfreopen.sh b/test/unittest/actions/freopen/tst.badfreopen.sh
index 21c890f2..f339e2e2 100755
--- a/test/unittest/actions/freopen/tst.badfreopen.sh
+++ b/test/unittest/actions/freopen/tst.badfreopen.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
script()
{
$dtrace $dt_flags -wq -o $tmpfile -s /dev/stdin 2> $errfile <<EOF
diff --git a/test/unittest/operators/tst.ternary.d b/test/unittest/operators/tst.ternary.d
index bcf0e3cb..343bc439 100644
--- a/test/unittest/operators/tst.ternary.d
+++ b/test/unittest/operators/tst.ternary.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/printf/tst.basics.d b/test/unittest/printf/tst.basics.d
index 14fec099..8da343ab 100644
--- a/test/unittest/printf/tst.basics.d
+++ b/test/unittest/printf/tst.basics.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/printf/tst.precs.d b/test/unittest/printf/tst.precs.d
index bac8a90f..0d08ae55 100644
--- a/test/unittest/printf/tst.precs.d
+++ b/test/unittest/printf/tst.precs.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/printf/tst.wp.d b/test/unittest/printf/tst.wp.d
index 019ca5a8..c72b518f 100644
--- a/test/unittest/printf/tst.wp.d
+++ b/test/unittest/printf/tst.wp.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/regression/tst.sigaltstack.d b/test/unittest/regression/tst.sigaltstack.d
index 48f0a4e7..15cb8011 100644
--- a/test/unittest/regression/tst.sigaltstack.d
+++ b/test/unittest/regression/tst.sigaltstack.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 */
#pragma D option destructive
#pragma D option quiet
diff --git a/test/unittest/scripting/tst.arg0.d b/test/unittest/scripting/tst.arg0.d
index 92e2756f..6819d40f 100644
--- a/test/unittest/scripting/tst.arg0.d
+++ b/test/unittest/scripting/tst.arg0.d
@@ -6,7 +6,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/scripting/tst.stringmacro.sh b/test/unittest/scripting/tst.stringmacro.sh
index 880fb242..4bd298e3 100755
--- a/test/unittest/scripting/tst.stringmacro.sh
+++ b/test/unittest/scripting/tst.stringmacro.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
############################################################################
# ASSERTION:
diff --git a/test/unittest/struct/tst.clauselocal.d b/test/unittest/struct/tst.clauselocal.d
index bb822ed9..3f745bbe 100644
--- a/test/unittest/struct/tst.clauselocal.d
+++ b/test/unittest/struct/tst.clauselocal.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 */
#pragma D option quiet
--
2.31.1
More information about the DTrace-devel
mailing list