Re: [PATCH 0/3] Fix strlen() overwriting strtab's read-only data and zeros

[Kris Van Hees <kris.van.hees@oracle.com>]

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>
>