* [PATCH 0/3] Fix strlen() overwriting strtab's read-only data and zeros
@ 2026-10-18 2:50 ` Eslam Almorshdy via B4 Relay
0 siblings, 0 replies; 9+ messages in thread
From: Eslam Almorshdy via B4 Relay @ 2026-10-18 2:50 UTC (permalink / raw)
To: dtrace; +Cc: Kris Van Hees, dtrace-devel, Eugene Loh, Eslam Almorshdy
strlen() copies its argument to the end of the string table in the
strtab map, which is where the block of zeros and the read-only data
have lived since commits d9d862b6 and e11098e2. So strlen() of
anything longer than a few characters overwrites them for the rest of
the session:
# dtrace -qn 'BEGIN { @[strlen("yyyyyyyyyyyyyyyyyyyyyyyyyyy")] =
count(); exit(0); }'
27 8753160913407277434
Patch 1 has the history and the other effects. Patch 3 is hardening
and can be dropped: after patch 1 no BPF code writes to strtab, and
patch 3 has the verifier enforce that.
Giving strlen() its own space back is a one-line alternative:
- dtp->dt_rooffset = P2ROUNDUP(dtp->dt_strlen, 8);
+ dtp->dt_rooffset = P2ROUNDUP(dtp->dt_strlen + strsize + 1, 8);
I chose the temporary string because it leaves strtab with no BPF
writer, which patch 3 relies on, and because with the one-liner all
CPUs write the same scratch bytes. With 8 threads calling getppid()
under a clause doing four strlen() of 255 bytes, the one-liner ran at
3.2-3.7M calls/s and patch 1 at 5.4-5.6M (both 1.26M with one thread).
The cost is one more temporary string held during the call, as
strchr() does. Happy to send a v2 with the one-liner if you prefer.
Tested on devel 61fea621, x86_64, Linux 6.8. Over funcs/, aggs/,
assocs/, variables/, printa/, multiaggs/, arrays/, codegen/ and dif/
(778 tests), the only change from patches 1-2 and 1-3 is the two new
tests passing. They also fail without patch 1 and pass with it on
dev-queue 0e93872f.
---
Eslam Almorshdy (3):
Use a temporary string as scratch space for strlen()
test: strlen() must not overwrite the rodata or the zeros
Create the strtab map read-only for BPF programs
bpf/strlen.c | 4 +-
libdtrace/dt_bpf.c | 19 ++++++---
libdtrace/dt_cg.c | 13 +++++--
test/unittest/funcs/strlen/tst.no-clobber-rodata.d | 37 ++++++++++++++++++
test/unittest/funcs/strlen/tst.no-clobber-rodata.r | 3 ++
test/unittest/funcs/strlen/tst.no-clobber-zeros.d | 45 ++++++++++++++++++++++
test/unittest/funcs/strlen/tst.no-clobber-zeros.r | 5 +++
7 files changed, 114 insertions(+), 12 deletions(-)
---
base-commit: 61fea621160ea4b6302874c62a723da105b44883
change-id: 20261018-bug010-strlen-strtab-1c4e6c7d2d06
Best regards,
--
Eslam Almorshdy <ealmorshdy@d-matrix.ai>
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/3] Use a temporary string as scratch space for strlen()
2026-10-18 2:50 ` Eslam Almorshdy via B4 Relay
@ 2026-10-18 2:50 ` Eslam Almorshdy via B4 Relay
-1 siblings, 0 replies; 9+ messages in thread
From: Eslam Almorshdy @ 2026-10-18 2:50 UTC (permalink / raw)
To: dtrace; +Cc: Kris Van Hees, dtrace-devel, Eugene Loh, Eslam Almorshdy
dt_strlen() measures its argument by copying it with
bpf_probe_read_str() to the end of the string table in the strtab map,
at offset STBSZ. Commit e0424837 ("Implement strlen() based on
bpf_probe_read_str()") started using that space, which was unused and
sized to hold the largest string. Commit d9d862b6 ("Add a block of
zeroes to use to initialize BPF maps") then put the block of zeros
there, rounded up to a multiple of 8, and commit e11098e2 ("Add rodata
support") put the read-only data in front of it. So strlen() of
anything longer than a few characters overwrites the read-only data,
or the block of zeros if there is none, for the rest of the session.
New aggregation data is initialized from the block of zeros and tuple
keys are padded from it. So after such a strlen(), aggregations such
as @[strlen(probefunc)] = count() give counts like 8753160913407277434,
and keys no longer match the same keys built before it. inet_ntoa6()
reads a lookup table from the read-only data and formats ::1 as
0:0:0:0:0:0:0:1.
Copy the string to a temporary string instead, as the other string
subroutines do, so that BPF code no longer writes to the strtab map.
Assisted-by: Claude Code:claude-opus-5-5
Signed-off-by: Eslam Almorshdy <ealmorshdy@d-matrix.ai>
---
bpf/strlen.c | 4 +---
libdtrace/dt_cg.c | 13 ++++++++++---
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/bpf/strlen.c b/bpf/strlen.c
index 5408ed17..4714f44b 100644
--- a/bpf/strlen.c
+++ b/bpf/strlen.c
@@ -12,12 +12,10 @@
# define noinline __attribute__((noinline))
#endif
-extern uint64_t STBSZ;
extern uint64_t STRSZ;
-noinline uint64_t dt_strlen(const dt_dctx_t *dctx, const char *str)
+noinline uint64_t dt_strlen(const char *str, char *tmp)
{
- char *tmp = dctx->strtab + (uint64_t)&STBSZ;
int64_t len;
/*
diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index a77c022c..a58837e0 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -6448,6 +6448,7 @@ dt_cg_subr_strlen(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
{
dt_ident_t *idp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_strlen");
dt_node_t *str = dnp->dn_args;
+ uint64_t off;
assert(idp != NULL);
@@ -6459,14 +6460,20 @@ dt_cg_subr_strlen(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
if (dt_regset_xalloc_args(drp) == -1)
longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
- dt_cg_access_dctx(BPF_REG_1, dlp, drp, -1);
- emit(dlp, BPF_MOV_REG(BPF_REG_2, str->dn_reg));
+ emit(dlp, BPF_MOV_REG(BPF_REG_1, str->dn_reg));
dt_regset_free(drp, str->dn_reg);
- dt_cg_tstring_free(yypcb, str);
+
+ /* The string is copied to a temporary string to measure it. */
+ off = dt_cg_tstring_xalloc(yypcb);
+ dt_cg_access_dctx(BPF_REG_2, dlp, drp, DCTX_MEM);
+ emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, off));
+
dt_regset_xalloc(drp, BPF_REG_0);
emite(dlp,BPF_CALL_FUNC(idp->di_id), idp);
dt_regset_free_args(drp);
+ dt_cg_tstring_xfree(yypcb, off);
+ dt_cg_tstring_free(yypcb, str);
dnp->dn_reg = dt_regset_alloc(drp);
if (dnp->dn_reg == -1)
longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
--
2.55.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 1/3] Use a temporary string as scratch space for strlen()
@ 2026-10-18 2:50 ` Eslam Almorshdy via B4 Relay
0 siblings, 0 replies; 9+ messages in thread
From: Eslam Almorshdy via B4 Relay @ 2026-10-18 2:50 UTC (permalink / raw)
To: dtrace; +Cc: Kris Van Hees, dtrace-devel, Eugene Loh, Eslam Almorshdy
From: Eslam Almorshdy <ealmorshdy@d-matrix.ai>
dt_strlen() measures its argument by copying it with
bpf_probe_read_str() to the end of the string table in the strtab map,
at offset STBSZ. Commit e0424837 ("Implement strlen() based on
bpf_probe_read_str()") started using that space, which was unused and
sized to hold the largest string. Commit d9d862b6 ("Add a block of
zeroes to use to initialize BPF maps") then put the block of zeros
there, rounded up to a multiple of 8, and commit e11098e2 ("Add rodata
support") put the read-only data in front of it. So strlen() of
anything longer than a few characters overwrites the read-only data,
or the block of zeros if there is none, for the rest of the session.
New aggregation data is initialized from the block of zeros and tuple
keys are padded from it. So after such a strlen(), aggregations such
as @[strlen(probefunc)] = count() give counts like 8753160913407277434,
and keys no longer match the same keys built before it. inet_ntoa6()
reads a lookup table from the read-only data and formats ::1 as
0:0:0:0:0:0:0:1.
Copy the string to a temporary string instead, as the other string
subroutines do, so that BPF code no longer writes to the strtab map.
Assisted-by: Claude Code:claude-opus-5-5
Signed-off-by: Eslam Almorshdy <ealmorshdy@d-matrix.ai>
---
bpf/strlen.c | 4 +---
libdtrace/dt_cg.c | 13 ++++++++++---
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/bpf/strlen.c b/bpf/strlen.c
index 5408ed17..4714f44b 100644
--- a/bpf/strlen.c
+++ b/bpf/strlen.c
@@ -12,12 +12,10 @@
# define noinline __attribute__((noinline))
#endif
-extern uint64_t STBSZ;
extern uint64_t STRSZ;
-noinline uint64_t dt_strlen(const dt_dctx_t *dctx, const char *str)
+noinline uint64_t dt_strlen(const char *str, char *tmp)
{
- char *tmp = dctx->strtab + (uint64_t)&STBSZ;
int64_t len;
/*
diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index a77c022c..a58837e0 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -6448,6 +6448,7 @@ dt_cg_subr_strlen(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
{
dt_ident_t *idp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_strlen");
dt_node_t *str = dnp->dn_args;
+ uint64_t off;
assert(idp != NULL);
@@ -6459,14 +6460,20 @@ dt_cg_subr_strlen(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
if (dt_regset_xalloc_args(drp) == -1)
longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
- dt_cg_access_dctx(BPF_REG_1, dlp, drp, -1);
- emit(dlp, BPF_MOV_REG(BPF_REG_2, str->dn_reg));
+ emit(dlp, BPF_MOV_REG(BPF_REG_1, str->dn_reg));
dt_regset_free(drp, str->dn_reg);
- dt_cg_tstring_free(yypcb, str);
+
+ /* The string is copied to a temporary string to measure it. */
+ off = dt_cg_tstring_xalloc(yypcb);
+ dt_cg_access_dctx(BPF_REG_2, dlp, drp, DCTX_MEM);
+ emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, off));
+
dt_regset_xalloc(drp, BPF_REG_0);
emite(dlp,BPF_CALL_FUNC(idp->di_id), idp);
dt_regset_free_args(drp);
+ dt_cg_tstring_xfree(yypcb, off);
+ dt_cg_tstring_free(yypcb, str);
dnp->dn_reg = dt_regset_alloc(drp);
if (dnp->dn_reg == -1)
longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
--
2.55.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] test: strlen() must not overwrite the rodata or the zeros
2026-10-18 2:50 ` Eslam Almorshdy via B4 Relay
@ 2026-10-18 2:50 ` Eslam Almorshdy via B4 Relay
-1 siblings, 0 replies; 9+ messages in thread
From: Eslam Almorshdy @ 2026-10-18 2:50 UTC (permalink / raw)
To: dtrace; +Cc: Kris Van Hees, dtrace-devel, Eugene Loh, Eslam Almorshdy
Both tests call strlen() on a string longer than strsize, then use
what it used to overwrite. tst.no-clobber-zeros.d checks that keys
built before the call still find their elements and that a new
aggregation starts from zero. tst.no-clobber-rodata.d checks that
inet_ntoa6(), which reads a table from the read-only data, still
formats ::1. They are separate because when a program has read-only
data, it lies in front of the zeros and takes the whole overwrite.
Both fail without the previous patch.
Assisted-by: Claude Code:claude-opus-5-5
Signed-off-by: Eslam Almorshdy <ealmorshdy@d-matrix.ai>
---
test/unittest/funcs/strlen/tst.no-clobber-rodata.d | 37 ++++++++++++++++++
test/unittest/funcs/strlen/tst.no-clobber-rodata.r | 3 ++
test/unittest/funcs/strlen/tst.no-clobber-zeros.d | 45 ++++++++++++++++++++++
test/unittest/funcs/strlen/tst.no-clobber-zeros.r | 5 +++
4 files changed, 90 insertions(+)
diff --git a/test/unittest/funcs/strlen/tst.no-clobber-rodata.d b/test/unittest/funcs/strlen/tst.no-clobber-rodata.d
new file mode 100644
index 00000000..1b452fd5
--- /dev/null
+++ b/test/unittest/funcs/strlen/tst.no-clobber-rodata.d
@@ -0,0 +1,37 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2026, 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: strlen() does not overwrite the read-only data.
+ *
+ * SECTION: Actions and Subroutines/strlen()
+ */
+
+#pragma D option quiet
+#pragma D option strsize=32
+
+BEGIN
+{
+ this->ip6 = (struct in6_addr *)alloca(sizeof(struct in6_addr));
+ this->ip6->in6_u.u6_addr32[0] = 0;
+ this->ip6->in6_u.u6_addr32[1] = 0;
+ this->ip6->in6_u.u6_addr32[2] = 0;
+ this->ip6->in6_u.u6_addr32[3] = 0;
+ this->ip6->in6_u.u6_addr8[15] = 1;
+
+ /* Longer than strsize: strlen() copies as much as it can. */
+ printf("%d\n", strlen("0123456789012345678901234567890123456789"));
+
+ /* inet_ntoa6() uses a table in the read-only data. */
+ printf("%s\n", inet_ntoa6(this->ip6));
+ exit(0);
+}
+
+ERROR
+{
+ exit(1);
+}
diff --git a/test/unittest/funcs/strlen/tst.no-clobber-rodata.r b/test/unittest/funcs/strlen/tst.no-clobber-rodata.r
new file mode 100644
index 00000000..eb33a6be
--- /dev/null
+++ b/test/unittest/funcs/strlen/tst.no-clobber-rodata.r
@@ -0,0 +1,3 @@
+31
+::1
+
diff --git a/test/unittest/funcs/strlen/tst.no-clobber-zeros.d b/test/unittest/funcs/strlen/tst.no-clobber-zeros.d
new file mode 100644
index 00000000..5cd866e5
--- /dev/null
+++ b/test/unittest/funcs/strlen/tst.no-clobber-zeros.d
@@ -0,0 +1,45 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2026, 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: strlen() does not overwrite the block of zeros that tuple
+ * keys and new aggregations are initialized from.
+ *
+ * SECTION: Actions and Subroutines/strlen()
+ */
+
+#pragma D option quiet
+#pragma D option strsize=32
+
+BEGIN
+{
+ a["k"] = 1;
+ @s["k"] = sum(1);
+
+ /* Longer than strsize: strlen() copies as much as it can. */
+ printf("%d\n", strlen("0123456789012345678901234567890123456789"));
+
+ /* The same keys must find the same elements as before. */
+ printf("%d\n", a["k"]);
+ @s["k"] = sum(1);
+
+ /* A new aggregation must start from zero. */
+ @c = count();
+
+ exit(0);
+}
+
+END
+{
+ printa("%s %@d\n", @s);
+ printa("%@d\n", @c);
+}
+
+ERROR
+{
+ exit(1);
+}
diff --git a/test/unittest/funcs/strlen/tst.no-clobber-zeros.r b/test/unittest/funcs/strlen/tst.no-clobber-zeros.r
new file mode 100644
index 00000000..68fe1b8a
--- /dev/null
+++ b/test/unittest/funcs/strlen/tst.no-clobber-zeros.r
@@ -0,0 +1,5 @@
+31
+1
+k 2
+1
+
--
2.55.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/3] test: strlen() must not overwrite the rodata or the zeros
@ 2026-10-18 2:50 ` Eslam Almorshdy via B4 Relay
0 siblings, 0 replies; 9+ messages in thread
From: Eslam Almorshdy via B4 Relay @ 2026-10-18 2:50 UTC (permalink / raw)
To: dtrace; +Cc: Kris Van Hees, dtrace-devel, Eugene Loh, Eslam Almorshdy
From: Eslam Almorshdy <ealmorshdy@d-matrix.ai>
Both tests call strlen() on a string longer than strsize, then use
what it used to overwrite. tst.no-clobber-zeros.d checks that keys
built before the call still find their elements and that a new
aggregation starts from zero. tst.no-clobber-rodata.d checks that
inet_ntoa6(), which reads a table from the read-only data, still
formats ::1. They are separate because when a program has read-only
data, it lies in front of the zeros and takes the whole overwrite.
Both fail without the previous patch.
Assisted-by: Claude Code:claude-opus-5-5
Signed-off-by: Eslam Almorshdy <ealmorshdy@d-matrix.ai>
---
test/unittest/funcs/strlen/tst.no-clobber-rodata.d | 37 ++++++++++++++++++
test/unittest/funcs/strlen/tst.no-clobber-rodata.r | 3 ++
test/unittest/funcs/strlen/tst.no-clobber-zeros.d | 45 ++++++++++++++++++++++
test/unittest/funcs/strlen/tst.no-clobber-zeros.r | 5 +++
4 files changed, 90 insertions(+)
diff --git a/test/unittest/funcs/strlen/tst.no-clobber-rodata.d b/test/unittest/funcs/strlen/tst.no-clobber-rodata.d
new file mode 100644
index 00000000..1b452fd5
--- /dev/null
+++ b/test/unittest/funcs/strlen/tst.no-clobber-rodata.d
@@ -0,0 +1,37 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2026, 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: strlen() does not overwrite the read-only data.
+ *
+ * SECTION: Actions and Subroutines/strlen()
+ */
+
+#pragma D option quiet
+#pragma D option strsize=32
+
+BEGIN
+{
+ this->ip6 = (struct in6_addr *)alloca(sizeof(struct in6_addr));
+ this->ip6->in6_u.u6_addr32[0] = 0;
+ this->ip6->in6_u.u6_addr32[1] = 0;
+ this->ip6->in6_u.u6_addr32[2] = 0;
+ this->ip6->in6_u.u6_addr32[3] = 0;
+ this->ip6->in6_u.u6_addr8[15] = 1;
+
+ /* Longer than strsize: strlen() copies as much as it can. */
+ printf("%d\n", strlen("0123456789012345678901234567890123456789"));
+
+ /* inet_ntoa6() uses a table in the read-only data. */
+ printf("%s\n", inet_ntoa6(this->ip6));
+ exit(0);
+}
+
+ERROR
+{
+ exit(1);
+}
diff --git a/test/unittest/funcs/strlen/tst.no-clobber-rodata.r b/test/unittest/funcs/strlen/tst.no-clobber-rodata.r
new file mode 100644
index 00000000..eb33a6be
--- /dev/null
+++ b/test/unittest/funcs/strlen/tst.no-clobber-rodata.r
@@ -0,0 +1,3 @@
+31
+::1
+
diff --git a/test/unittest/funcs/strlen/tst.no-clobber-zeros.d b/test/unittest/funcs/strlen/tst.no-clobber-zeros.d
new file mode 100644
index 00000000..5cd866e5
--- /dev/null
+++ b/test/unittest/funcs/strlen/tst.no-clobber-zeros.d
@@ -0,0 +1,45 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2026, 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: strlen() does not overwrite the block of zeros that tuple
+ * keys and new aggregations are initialized from.
+ *
+ * SECTION: Actions and Subroutines/strlen()
+ */
+
+#pragma D option quiet
+#pragma D option strsize=32
+
+BEGIN
+{
+ a["k"] = 1;
+ @s["k"] = sum(1);
+
+ /* Longer than strsize: strlen() copies as much as it can. */
+ printf("%d\n", strlen("0123456789012345678901234567890123456789"));
+
+ /* The same keys must find the same elements as before. */
+ printf("%d\n", a["k"]);
+ @s["k"] = sum(1);
+
+ /* A new aggregation must start from zero. */
+ @c = count();
+
+ exit(0);
+}
+
+END
+{
+ printa("%s %@d\n", @s);
+ printa("%@d\n", @c);
+}
+
+ERROR
+{
+ exit(1);
+}
diff --git a/test/unittest/funcs/strlen/tst.no-clobber-zeros.r b/test/unittest/funcs/strlen/tst.no-clobber-zeros.r
new file mode 100644
index 00000000..68fe1b8a
--- /dev/null
+++ b/test/unittest/funcs/strlen/tst.no-clobber-zeros.r
@@ -0,0 +1,5 @@
+31
+1
+k 2
+1
+
--
2.55.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] Create the strtab map read-only for BPF programs
2026-10-18 2:50 ` Eslam Almorshdy via B4 Relay
@ 2026-10-18 2:50 ` Eslam Almorshdy via B4 Relay
-1 siblings, 0 replies; 9+ messages in thread
From: Eslam Almorshdy @ 2026-10-18 2:50 UTC (permalink / raw)
To: dtrace; +Cc: Kris Van Hees, dtrace-devel, Eugene Loh, Eslam Almorshdy
The strtab map holds the string table, the read-only data and the
block of zeros, and every BPF reader assumes they never change. Now
that strlen() uses a temporary string, no BPF code writes to the map,
but nothing enforced that, which is how the strlen() overwrite went
unnoticed.
Create the map with BPF_F_RDONLY_PROG, so that the verifier rejects a
program that writes to it. User space still fills the map, which the
flag does not restrict. BPF_F_RDONLY_PROG has been supported since
Linux 5.2.
The miscompiled strjoin() in codegen/tst.reg_spilling7.d (XFAIL) is an
example: it is now rejected at its first write into strtab, instead of
only when a later write runs past the end of the map.
Assisted-by: Claude Code:claude-opus-5-5
Signed-off-by: Eslam Almorshdy <ealmorshdy@d-matrix.ai>
---
libdtrace/dt_bpf.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
index 146a66e3..349f864a 100644
--- a/libdtrace/dt_bpf.c
+++ b/libdtrace/dt_bpf.c
@@ -548,8 +548,8 @@ map_create_error(dtrace_hdl_t *dtp, const char *name, int err)
}
static int
-create_gmap(dtrace_hdl_t *dtp, const char *name, enum bpf_map_type type,
- size_t ksz, size_t vsz, size_t size)
+create_gmap_flags(dtrace_hdl_t *dtp, const char *name, enum bpf_map_type type,
+ size_t ksz, size_t vsz, size_t size, uint32_t flags)
{
int fd, err;
dt_ident_t *idp;
@@ -561,7 +561,7 @@ create_gmap(dtrace_hdl_t *dtp, const char *name, enum bpf_map_type type,
fd = -1;
err = E2BIG;
} else {
- fd = dt_bpf_map_create(type, name, ksz, vsz, size, 0);
+ fd = dt_bpf_map_create(type, name, ksz, vsz, size, flags);
err = errno;
}
@@ -584,6 +584,13 @@ create_gmap(dtrace_hdl_t *dtp, const char *name, enum bpf_map_type type,
return fd;
}
+static int
+create_gmap(dtrace_hdl_t *dtp, const char *name, enum bpf_map_type type,
+ size_t ksz, size_t vsz, size_t size)
+{
+ return create_gmap_flags(dtp, name, type, ksz, vsz, size, 0);
+}
+
static int
create_gmap_of_maps(dtrace_hdl_t *dtp, const char *name,
enum bpf_map_type otype, size_t oksz, size_t osize,
@@ -848,7 +855,7 @@ gmap_create_scratchmem(dtrace_hdl_t *dtp)
* from the DTrace consumer handle (dt_strlen).
*
* The read-only data is appended to the string table, as is a memory block of
- * zeros for initializing memory regions.
+ * zeros for initializing memory regions. BPF programs may only read this map.
*
* In order to ensure the BPF verifier can validate all access requests for
* dynamic references to string constants, the size of the read-only data plus
@@ -914,8 +921,8 @@ gmap_create_strtab(dtrace_hdl_t *dtp)
dt_rodata_write(dtp->dt_rodata, (dt_rodata_copy_f *)dt_rodata_copy,
strtab + dtp->dt_rooffset);
- fd = create_gmap(dtp, "strtab", BPF_MAP_TYPE_ARRAY, sizeof(uint32_t),
- sz, 1);
+ fd = create_gmap_flags(dtp, "strtab", BPF_MAP_TYPE_ARRAY,
+ sizeof(uint32_t), sz, 1, BPF_F_RDONLY_PROG);
if (fd == -1)
return -1;
--
2.55.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/3] Create the strtab map read-only for BPF programs
@ 2026-10-18 2:50 ` Eslam Almorshdy via B4 Relay
0 siblings, 0 replies; 9+ messages in thread
From: Eslam Almorshdy via B4 Relay @ 2026-10-18 2:50 UTC (permalink / raw)
To: dtrace; +Cc: Kris Van Hees, dtrace-devel, Eugene Loh, Eslam Almorshdy
From: Eslam Almorshdy <ealmorshdy@d-matrix.ai>
The strtab map holds the string table, the read-only data and the
block of zeros, and every BPF reader assumes they never change. Now
that strlen() uses a temporary string, no BPF code writes to the map,
but nothing enforced that, which is how the strlen() overwrite went
unnoticed.
Create the map with BPF_F_RDONLY_PROG, so that the verifier rejects a
program that writes to it. User space still fills the map, which the
flag does not restrict. BPF_F_RDONLY_PROG has been supported since
Linux 5.2.
The miscompiled strjoin() in codegen/tst.reg_spilling7.d (XFAIL) is an
example: it is now rejected at its first write into strtab, instead of
only when a later write runs past the end of the map.
Assisted-by: Claude Code:claude-opus-5-5
Signed-off-by: Eslam Almorshdy <ealmorshdy@d-matrix.ai>
---
libdtrace/dt_bpf.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
index 146a66e3..349f864a 100644
--- a/libdtrace/dt_bpf.c
+++ b/libdtrace/dt_bpf.c
@@ -548,8 +548,8 @@ map_create_error(dtrace_hdl_t *dtp, const char *name, int err)
}
static int
-create_gmap(dtrace_hdl_t *dtp, const char *name, enum bpf_map_type type,
- size_t ksz, size_t vsz, size_t size)
+create_gmap_flags(dtrace_hdl_t *dtp, const char *name, enum bpf_map_type type,
+ size_t ksz, size_t vsz, size_t size, uint32_t flags)
{
int fd, err;
dt_ident_t *idp;
@@ -561,7 +561,7 @@ create_gmap(dtrace_hdl_t *dtp, const char *name, enum bpf_map_type type,
fd = -1;
err = E2BIG;
} else {
- fd = dt_bpf_map_create(type, name, ksz, vsz, size, 0);
+ fd = dt_bpf_map_create(type, name, ksz, vsz, size, flags);
err = errno;
}
@@ -584,6 +584,13 @@ create_gmap(dtrace_hdl_t *dtp, const char *name, enum bpf_map_type type,
return fd;
}
+static int
+create_gmap(dtrace_hdl_t *dtp, const char *name, enum bpf_map_type type,
+ size_t ksz, size_t vsz, size_t size)
+{
+ return create_gmap_flags(dtp, name, type, ksz, vsz, size, 0);
+}
+
static int
create_gmap_of_maps(dtrace_hdl_t *dtp, const char *name,
enum bpf_map_type otype, size_t oksz, size_t osize,
@@ -848,7 +855,7 @@ gmap_create_scratchmem(dtrace_hdl_t *dtp)
* from the DTrace consumer handle (dt_strlen).
*
* The read-only data is appended to the string table, as is a memory block of
- * zeros for initializing memory regions.
+ * zeros for initializing memory regions. BPF programs may only read this map.
*
* In order to ensure the BPF verifier can validate all access requests for
* dynamic references to string constants, the size of the read-only data plus
@@ -914,8 +921,8 @@ gmap_create_strtab(dtrace_hdl_t *dtp)
dt_rodata_write(dtp->dt_rodata, (dt_rodata_copy_f *)dt_rodata_copy,
strtab + dtp->dt_rooffset);
- fd = create_gmap(dtp, "strtab", BPF_MAP_TYPE_ARRAY, sizeof(uint32_t),
- sz, 1);
+ fd = create_gmap_flags(dtp, "strtab", BPF_MAP_TYPE_ARRAY,
+ sizeof(uint32_t), sz, 1, BPF_F_RDONLY_PROG);
if (fd == -1)
return -1;
--
2.55.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] Fix strlen() overwriting strtab's read-only data and zeros
2026-10-18 2:50 ` Eslam Almorshdy via B4 Relay
` (3 preceding siblings ...)
(?)
@ 2026-10-30 23:26 ` Kris Van Hees
-1 siblings, 0 replies; 9+ messages in thread
From: Kris Van Hees @ 2026-10-30 23:26 UTC (permalink / raw)
To: ealmorshdy; +Cc: dtrace, Kris Van Hees, dtrace-devel
Hi,
Just a quick message that I am reviewing your patches. Thank you so much for
posting them. You definitely identified some genuine bugs and propose very
good solutions for fixing them. I really appreciate your contributing to
DTrace.
Please use the dtrace@lists.linux.dev list going forward for all patches and
correspondence, as we are using that one as the primary public mailing list
for DTrace.
Thanks,
Kris
On Sun, Oct 18, 2026 at 02:50:06AM +0000, Eslam Almorshdy via B4 Relay wrote:
> strlen() copies its argument to the end of the string table in the
> strtab map, which is where the block of zeros and the read-only data
> have lived since commits d9d862b6 and e11098e2. So strlen() of
> anything longer than a few characters overwrites them for the rest of
> the session:
>
> # dtrace -qn 'BEGIN { @[strlen("yyyyyyyyyyyyyyyyyyyyyyyyyyy")] =
> count(); exit(0); }'
>
> 27 8753160913407277434
>
> Patch 1 has the history and the other effects. Patch 3 is hardening
> and can be dropped: after patch 1 no BPF code writes to strtab, and
> patch 3 has the verifier enforce that.
>
> Giving strlen() its own space back is a one-line alternative:
>
> - dtp->dt_rooffset = P2ROUNDUP(dtp->dt_strlen, 8);
> + dtp->dt_rooffset = P2ROUNDUP(dtp->dt_strlen + strsize + 1, 8);
>
> I chose the temporary string because it leaves strtab with no BPF
> writer, which patch 3 relies on, and because with the one-liner all
> CPUs write the same scratch bytes. With 8 threads calling getppid()
> under a clause doing four strlen() of 255 bytes, the one-liner ran at
> 3.2-3.7M calls/s and patch 1 at 5.4-5.6M (both 1.26M with one thread).
> The cost is one more temporary string held during the call, as
> strchr() does. Happy to send a v2 with the one-liner if you prefer.
>
> Tested on devel 61fea621, x86_64, Linux 6.8. Over funcs/, aggs/,
> assocs/, variables/, printa/, multiaggs/, arrays/, codegen/ and dif/
> (778 tests), the only change from patches 1-2 and 1-3 is the two new
> tests passing. They also fail without patch 1 and pass with it on
> dev-queue 0e93872f.
>
> ---
> Eslam Almorshdy (3):
> Use a temporary string as scratch space for strlen()
> test: strlen() must not overwrite the rodata or the zeros
> Create the strtab map read-only for BPF programs
>
> bpf/strlen.c | 4 +-
> libdtrace/dt_bpf.c | 19 ++++++---
> libdtrace/dt_cg.c | 13 +++++--
> test/unittest/funcs/strlen/tst.no-clobber-rodata.d | 37 ++++++++++++++++++
> test/unittest/funcs/strlen/tst.no-clobber-rodata.r | 3 ++
> test/unittest/funcs/strlen/tst.no-clobber-zeros.d | 45 ++++++++++++++++++++++
> test/unittest/funcs/strlen/tst.no-clobber-zeros.r | 5 +++
> 7 files changed, 114 insertions(+), 12 deletions(-)
> ---
> base-commit: 61fea621160ea4b6302874c62a723da105b44883
> change-id: 20261018-bug010-strlen-strtab-1c4e6c7d2d06
>
> Best regards,
> --
> Eslam Almorshdy <ealmorshdy@d-matrix.ai>
>
^ permalink raw reply [flat|nested] 9+ messages in thread