[Bug runtime/31074] New: On aarch64 the systemtap.base/set_kernel.stp triggers "Unable to handle kernel paging request"

[Bug runtime/31074] New: On aarch64 the systemtap.base/set_kernel.stp triggers "Unable to handle kernel paging request"

[wcohen at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31074

            Bug ID: 31074
           Summary: On aarch64 the systemtap.base/set_kernel.stp triggers
                    "Unable to handle kernel paging request"
           Product: systemtap
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: runtime
          Assignee: systemtap at sourceware dot org
          Reporter: wcohen at redhat dot com
  Target Milestone: ---

When attempting to run the systemtap tests on aarch64 fedora 39 the
systemtap.base/set_kernel.stp has an "Unable to handle kernel access" when the
script is shutting down. On the machine have a git checkout of systemtap built
as an rpm and a recent f39 kernel:

$ rpm -q systemtap kernel
systemtap-5.0-1.202311152024.fc39.aarch64
kernel-6.5.11-300.fc39.aarch64

This can be demonstrated with on aarch64 machine with the following commands:

$ stap -k -p4 -mset_kernel -v  -g set_kernel.stp
$ sudo staprun set_kernel.ko -c "sleep 1"

It may take a couple tries of staprun to trigger and get:

[  764.982599] Unable to handle kernel access to user memory outside uaccess
routines at virtual address 0000000000000030
[  764.993402] Mem abort info:
[  764.996253]   ESR = 0x0000000096000004
[  765.000063]   EC = 0x25: DABT (current EL), IL = 32 bits
[  765.005419]   SET = 0, FnV = 0
[  765.008489]   EA = 0, S1PTW = 0
[  765.011636]   FSC = 0x04: level 0 translation fault
[  765.016527] Data abort info:
[  765.019412]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[  765.024895]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[  765.029959]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[  765.035284] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000171423000
[  765.041737] [0000000000000030] pgd=0000000000000000, p4d=0000000000000000
[  765.048549] Internal error: Oops: 0000000096000004 [#1] SMP
[  765.054121] Modules linked in: set_kernel(OE) snd_seq_dummy snd_hrtimer
nf_conntrack_netbios_ns nf_conntrack_broadcast nft_fib_inett
[  765.054381]  usb_conn_gpio udc_core tegra_soctherm snd_timer videodev at24
snd vfat soundcore mc fat loop zram mmc_block onboard_use
[  765.144548] Unloaded tainted modules: set_kernel(OE):1 [last unloaded:
set_kernel(OE)]
[  765.190112] CPU: 2 PID: 5917 Comm: stapio Tainted: G        WC OE     
6.5.11-300.fc39.aarch64 #1
[  765.198979] Hardware name: nvidia p3450-0000/p3450-0000, BIOS 2020.10
10/06/2020
[  765.206365] pstate: 00400005 (nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  765.213320] pc : __pi_strcmp+0xbc/0x140
[  765.217173] lr : get_tracepoint+0x5c/0x80 [set_kernel]
[  765.222348] sp : ffff8000871c3a30
[  765.225656] x29: ffff8000871c3a30 x28: ffff800082bcf850 x27:
ffff80007c3ddc80
[  765.232791] x26: ffff00008423d560 x25: dead000000000122 x24:
dead000000000100
[  765.239923] x23: 000000000000000b x22: ffff80007c3dd298 x21:
ffff000089e41c20
[  765.247055] x20: ffff80007be10610 x19: 0000000000000001 x18:
0000000000000000
[  765.254189] x17: 000000040044ffff x16: 00500074b5503510 x15:
0000000000000000
[  765.261321] x14: ffff00008035a200 x13: ffff80007c2db000 x12:
ffff800082bcf6d8
[  765.268452] x11: 0000000000000001 x10: 00007fff841efa21 x9 :
fffffffffffffe78
[  765.275583] x8 : 0101010101010101 x7 : 000000002ad85bff x6 :
0000000000000000
[  765.282716] x5 : 676461675f627375 x4 : 0000000000000000 x3 :
0000000000000000
[  765.289848] x2 : 00000000000000ea x1 : 0000000000000030 x0 :
ffff80007be10610
[  765.296984] Call trace:
[  765.299432]  __pi_strcmp+0xbc/0x140
[  765.302926]  stp_tracepoint_notify+0x7c/0x248 [set_kernel]
[  765.308431]  unregister_tracepoint_module_notifier+0x6c/0xa8
[  765.314097]  stp_tracepoint_exit+0x40/0xc8 [set_kernel]
[  765.319337]  systemtap_module_exit+0x1bc/0x2f0 [set_kernel]
[  765.324922]  _stp_cleanup_and_exit.part.0+0xe8/0x128 [set_kernel]
[  765.331025]  _stp_ctl_write_cmd+0x184/0x4e8 [set_kernel]
[  765.336344]  proc_reg_write+0xa4/0x100
[  765.340104]  vfs_write+0xd0/0x318
[  765.343421]  ksys_write+0x7c/0x120
[  765.346823]  __arm64_sys_write+0x24/0x38
[  765.350743]  invoke_syscall+0x78/0x100
[  765.354493]  el0_svc_common.constprop.0+0x4c/0xf8
[  765.359191]  do_el0_svc+0x34/0x50
[  765.362502]  el0_svc+0x34/0x108
[  765.365650]  el0t_64_sync_handler+0x120/0x130
[  765.370003]  el0t_64_sync+0x194/0x198
[  765.373673] Code: f240081f 54ffff41 cb010fe9 927df021 (f8408427)
[  765.379763] ---[ end trace 0000000000000000 ]---


I suspect that some how the buffer in get_buffer is overlapping other memory
somehow.  Doubling the buffersize, but limiting the memset to MAXTRINGLEN seems
to eliminate problem.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/31074] On aarch64 the systemtap.base/set_kernel.stp triggers "Unable to handle kernel paging request"

[fche at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31074

Frank Ch. Eigler <fche at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fche at redhat dot com

--- Comment #1 from Frank Ch. Eigler <fche at redhat dot com> ---
That failing strcmp may come from stp_tracepoint.c, via

stp_tracepoint_going / get_tracepoint

00126         head = &tracepoint_table[hash & (TRACEPOINT_TABLE_SIZE - 1)];     
00127         hlist_for_each_entry(e, head, hlist) {                            
00128                 if (!strcmp(name, e->name))                               
00129                         return e;                                         
00130         }                                                                 

that offset 0x30 looks like it could be a match for e->name with null e

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/31074] On aarch64 the systemtap.base/set_kernel.stp triggers "Unable to handle kernel paging request"

[wcohen at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31074

William Cohen <wcohen at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #2 from William Cohen <wcohen at redhat dot com> ---
Yes, it looked like the value might NULL + offset for field.  Added "-g
-save-temps" to EXTRA_CFLAGS of the make file of the save /tmp/stap* to get a
better idea of what the compiler is generating for code and where things are
located.

for the get_buffer:
        adrp    x3, .LANCHOR0
        add     x3, x3, :lo12:.LANCHOR0
        add     x3, x3, 568
        mov     x2, 512
        mov     x0, x3
        mov     w1, 0
.LVL175:
        .loc 23 1963 178 discriminator 1 view .LVU775
        bl      memset

get_tracepoint (static tracepoint_table)

.LVL489:
        .loc 27 127 36 view .LVU2068
        adrp    x1, .LANCHOR0
        add     x1, x1, :lo12:.LANCHOR0
        add     x1, x1, 1080
        ldr     x19, [x1, x0, lsl 3]

One thought that crossed my mind is that the memset code is pretty optimized
using cacheline zeroing for specific memset(x, 0, size) operations and might be
overrunning the end the static buffer into the static tracepoint_table as
buffer is not aligned to cache boundaries.  However, reducing the size of the
get_buffer memset clearing didn't eliminate the problem.

Thinking probably should add some diagnostics to the stp_tracepoint.c to get a
better understanding how tracepoint_table entries are getting corrupted.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/31074] On aarch64 the systemtap.base/set_kernel.stp triggers "Unable to handle kernel paging request"

[fche at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31074

--- Comment #3 from Frank Ch. Eigler <fche at redhat dot com> ---
... and as a temporary measure, could consider

diff --git a/runtime/linux/stp_tracepoint.c b/runtime/linux/stp_tracepoint.c
index 508948dce4fd..9b7409194956 100644
--- a/runtime/linux/stp_tracepoint.c
+++ b/runtime/linux/stp_tracepoint.c
@@ -286,7 +286,10 @@ int stp_tracepoint_coming(struct tp_module *tp_mod)
                #else
                tp = tp_mod->mod->tracepoints_ptrs[i];
                #endif
-               e = get_tracepoint(tp->name);
+                if (!tp) {
+                        WARN_ON(!tp);
+                        continue;
+                }
                if (!e) {
                        e = add_tracepoint(tp->name);
                        if (IS_ERR(e)) {

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/31074] On aarch64 the systemtap.base/set_kernel.stp triggers "Unable to handle kernel paging request"

[wcohen at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31074

--- Comment #4 from William Cohen <wcohen at redhat dot com> ---
Looking a bit more at the stp_tracepoint.c code.  If e was NULL for
hlist_for_each_entry should, it should exit the for loop rather than doing the
strcmp:

https://elixir.bootlin.com/linux/v6.5.11/source/include/linux/list.h#L1053

The problem attemtped access is occurring when tracepoints are being removed. 
It is a bit surprising the that the similar code in add_tracepoint doesn't
encounter a similar problem earlier around with virtually identical code in
add_tracepoint:

https://sourceware.org/git/?p=systemtap.git;a=blob;f=runtime/linux/stp_tracepoint.c;h=508948dce4fd438bde6d9d155035faba2abd0ee1;hb=HEAD#l146



The suggested patch isn't making much sense to me. 

e is a local variable that would no longer be initialized before the
"if(!e){..." check.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/31074] On aarch64 the systemtap.base/set_kernel.stp triggers "Unable to handle kernel paging request"

[wcohen at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31074

--- Comment #5 from William Cohen <wcohen at redhat dot com> ---
I have done some addition investigation using diagnostic print statements to
runtime/linux/stp_tracepoint.c.  The tracepoint_table[0] is getting corrupted. 
The lower 4 bytes of that entry are being zeroed. 

The problem is linked to the:

 set_kernel_string(addr, "foobar")

The interesting thing is that the set_kernel_string_n() functions later in the
test do not have an issue, but they use the same underlying code.  If the len
passed in is set to a value greater than 508 then the set_kernel_string_n()
will cause a similar corruption.  So it appears that issue is happening
_stp_store_deref_string.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/31074] On aarch64 the systemtap.base/set_kernel.stp triggers "Unable to handle kernel paging request"

[wcohen at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31074

--- Comment #6 from William Cohen <wcohen at redhat dot com> ---
The code in loc2c-runtime.c  _stp_store_deref_string_() looks like it can write
past the end of the buffer.  The for loop has a loop test of "i < len-1".  When
the for loop exits i == len. The statement after the for loop is:

  err = __stp_put_either('\0', (u8 *)addr + i, seg);

This would be effectively addressing addr + len.  The ranges accessing the
buffer should be 0 to len-1.

For aarch64 the buffer for set_kernel.stp get_buffer function and the
tracepoint_table butt up to each other no padding between them.  Some
tracepoints on aarch64 map to the tracepoint_table[0] which is getting
corrupted.  On x86_64 either nothing is getting mapped to tracepoint_table[0]
or there is some space between the end of buffer and the beginning of
tracepoint_table.

On aarch64 sizeof('\0') is returning 4 rather than 1 as expected for a
character. This would explain 4 bytes of tracepoint_table[0] being 0.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/31074] On aarch64 the systemtap.base/set_kernel.stp triggers "Unable to handle kernel paging request"

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31074

Mark Wielaard <mark at klomp dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mark at klomp dot org

--- Comment #7 from Mark Wielaard <mark at klomp dot org> ---
(In reply to William Cohen from comment #6)
> On aarch64 sizeof('\0') is returning 4 rather than 1 as expected for a
> character.

That is actually so on all arches. The confusing thing is that a character
constant has type int. Which explains why sizeof ('a') is really sizeof (int)
== 4. While char c = 'a'; sizeof (c) == 1 (sizeof (char)).

Note that C and C++ differ here. In C++ character constants are type char and
so this program:

#include <stdio.h>

int
main ()
{
  printf ("sizeof literal char: %d\n", sizeof ('a'));
}

$ gcc -g -o c c.c
$ ./c 
sizeof literal char: 4
$ g++ -g -o c c.c
$ ./c 
sizeof literal char: 1

Aren't C and C++ fun? :)

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/31074] On aarch64 the systemtap.base/set_kernel.stp triggers "Unable to handle kernel paging request"

[wcohen at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31074

William Cohen <wcohen at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #8 from William Cohen <wcohen at redhat dot com> ---
Fixed by

commit b84a5e8c2c5a857c0790a71df7824259a95131cf (HEAD -> master, origin/master,
origin/HEAD)
Author: William Cohen <wcohen@redhat.com>
Date:   Mon Dec 4 11:28:10 2023 -0500

    PR31074: Ensure that the set_kernel_string* functions limit their writes

    Both the set_kernel_string and set_kernel_string_n function use the
    underlying _stp_store_deref_string_ function to write strings.  There
    were two issues with the this function:

     1) wrote MAXSTRINGLEN bytes even if string was shorter
     2) null write at end could spill past end of buffer

    The first issue was addressed by stopping to write once a null
    character is encountered.  The second issue is a side effect of C
    implicit promotion of character constants to ints and was addressed by
    explicitlying casting the character constants as a char.

    The pr31074.exp test was added to verify that the write length are
    limited to string length and the null write does not go beyond the end
    of the buffer.

-- 
You are receiving this mail because:
You are the assignee for the bug.