Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libbpf-tools/softirqs: Fix logarithmic calculation issue #5208

Merged
merged 1 commit into from
Feb 11, 2025

Conversation

kknjh
Copy link
Contributor

@kknjh kknjh commented Feb 7, 2025

Should use log2l.
When there is a function call, R1 unbounded memory access problem occurs, so inline is used.

@kknjh
Copy link
Contributor Author

kknjh commented Feb 8, 2025

I encountered R1 unbounded memory access when changing log2 to log2l. The log is in softirq.log.
And there are other modifications that do not affect the code logic to avoid this problem, such as the following modifications

 if (slot >= MAX_SLOTS) {
            slot = MAX_SLOTS - 1;
            __sync_fetch_and_add(&hist->slots[slot], 1);
    }
    else
            __sync_fetch_and_add(&hist->slots[slot], 1);

The logs of both are as follows:

success:
    3: (67) r6 <<= 32                     ; R6_w=scalar(smax=0x7fffffff00000000,umax=0xffffffff00000000,smin32=0,smax32=umax32=0,var_off=(0x0; 0xffffffff00000000))
    4: (77) r6 >>= 32                     ; R6_w=scalar(smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff))
    ; if (vec_nr >= NR_SOFTIRQS)
    5: (25) if r6 > 0x9 goto pc+60        ; R6_w=scalar(smin=smin32=0,smax=umax=smax32=umax32=9,var_off=(0x0; 0xf))
    ......
    13: (18) r1 = 0xffffb2af418320a0      ; R1_w=map_value(map=softirqs.bss,ks=4,vs=960,off=160)
    15: (0f) r1 += r6                     ; R1_w=map_value(map=softirqs.bss,ks=4,vs=960,off=160,smin=smin32=0,smax=umax=smax32=umax32=720,var_off=(0x0; 0x3f0)) R6_w=scalar(smin=smin32=0,smax=umax=smax32=umax32=720,var_off=(0x0; 0x3f0))
    
Failed:
    7: (bf) r1 = r6                       ; frame1: R1_w=scalar(id=1) R6_w=scalar(id=1)
    8: (67) r1 <<= 32                     ; frame1: R1_w=scalar(smax=0x7fffffff00000000,umax=0xffffffff00000000,smin32=0,smax32=umax32=0,var_off=(0x0; 0xffffffff00000000))
    9: (77) r1 >>= 32                     ; frame1: R1_w=scalar(smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff))
    ; if (vec_nr >= NR_SOFTIRQS)
    10: (25) if r1 > 0x9 goto pc+89       ; frame1: R1_w=scalar(smin=smin32=0,smax=umax=smax32=umax32=9,var_off=(0x0; 0xf))
    ......
    46: (18) r1 = 0xffffba0f8095a0a0      ; frame1: R1_w=map_value(map=softirqs.bss,ks=4,vs=960,off=160)
    80: (0f) r1 += r6                     ; frame1: R1_w=map_value(map=softirqs.bss,ks=4,vs=960,off=160,smin=0,smax=umax=0x4fffffffb0,smax32=0x7ffffff0,umax32=0xfffffff0,var_off=(0x0; 0x7ffffffff0)) R6=scalar(smin=0,smax=umax=0x4fffffffb0,smax32=0x7ffffff0,umax32=0xfffffff0,var_off=(0x0; 0x7ffffffff0))

r1: hists
r6: vec_nr
r1+r6: hists[vec_nr]
Normally, after r6 determines the maximum value smax=umax=smax32=umax32=9, r1+r6 also limits r1.
The reason for the failure is that r6 was passed to r1, causing r6 to not determine the maximum value.
Subsequently, r1+r6 was used, resulting in R1 unbounded memory access
From the logs, it appears that this issue occurred during the function call process, so I used inline instead

However, may I ask why this situation has occurred?
softirq.log

@yonghong-song
Copy link
Collaborator

The failed code is due to compiler optimizations.
For the failed code:

7: (bf) r1 = r6                       ; frame1: R1_w=scalar(id=1) R6_w=scalar(id=1)
    8: (67) r1 <<= 32                     ; frame1: R1_w=scalar(smax=0x7fffffff00000000,umax=0xffffffff00000000,smin32=0,smax32=umax32=0,var_off=(0x0; 0xffffffff00000000))
    9: (77) r1 >>= 32                     ; frame1: R1_w=scalar(smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff))
    ; if (vec_nr >= NR_SOFTIRQS)
    10: (25) if r1 > 0x9 goto pc+89       ; frame1: R1_w=scalar(smin=smin32=0,smax=umax=smax32=umax32=9,var_off=(0x0; 0xf))
    ......
    46: (18) r1 = 0xffffba0f8095a0a0      ; frame1: R1_w=map_value(map=softirqs.bss,ks=4,vs=960,off=160)
    80: (0f) r1 += r6                     ; frame1: R1_w=map_value(map=softirqs.bss,ks=4,vs=960,off=160,smin=0,smax=umax=0x4fffffffb0,smax32=0x7ffffff0,umax32=0xfffffff0,var_off=(0x0; 0x7ffffffff0)) R6=scalar(smin=0,smax=umax=0x4fffffffb0,smax32=0x7ffffff0,umax32=0xfffffff0,var_off=(0x0; 0x7ffffffff0))

R1 did some range refinement. But unfortunately, register R6 didn't inherit refined R1 so later it caused verification.

In Makefile, we have

  BPFCFLAGS_softirqs := $(BPFCFLAGS) -mcpu=v3

why you didn't use -mcpu=v3? Which compiler you are using?

@kknjh
Copy link
Contributor Author

kknjh commented Feb 10, 2025

I'm so sorry. I happened to not update the patch 'libbpf tools/softirqs: Add - c/-- cpu option to filter CPU'. It's normal now. I removed the inline of this patch

@yonghong-song yonghong-song merged commit f954eb1 into iovisor:master Feb 11, 2025
1 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants