| Message ID | 20220429014240.3434866-2-pulehui@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support riscv jit to provide | expand |
Pu Lehui wrote: > We found that 32-bit environment can not print bpf line info due > to data inconsistency between jited_ksyms[0] and jited_linfo[0]. > > For example: > jited_kyms[0] = 0xb800067c, jited_linfo[0] = 0xffffffffb800067c > > We know that both of them store bpf func address, but due to the > different data extension operations when extended to u64, they may > not be the same. We need to unify the data extension operations of > them. > > Signed-off-by: Pu Lehui <pulehui@huawei.com> > --- > kernel/bpf/syscall.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > index e9e3e49c0eb7..18137ea5190d 100644 > --- a/kernel/bpf/syscall.c > +++ b/kernel/bpf/syscall.c > @@ -3871,13 +3871,16 @@ static int bpf_prog_get_info_by_fd(struct file *file, > info.nr_jited_line_info = 0; > if (info.nr_jited_line_info && ulen) { > if (bpf_dump_raw_ok(file->f_cred)) { > + unsigned long jited_linfo_addr; > __u64 __user *user_linfo; > u32 i; > > user_linfo = u64_to_user_ptr(info.jited_line_info); > ulen = min_t(u32, info.nr_jited_line_info, ulen); > for (i = 0; i < ulen; i++) { > - if (put_user((__u64)(long)prog->aux->jited_linfo[i], > + jited_linfo_addr = (unsigned long) > + prog->aux->jited_linfo[i]; > + if (put_user((__u64) jited_linfo_addr, > &user_linfo[i])) the logic is fine but i'm going to nitpick a bit this 4 lines is ugly just make it slightly longer than 80chars or use a shoarter name? For example, for (i = 0; i < ulen; i++) { unsigned long l; l = (unsigned long) prog->aux->jited_linfo[i]; if (put_user((__u64) l, &user_linfo[i])) is much nicer -- no reason to smash single assignment across multiple lines. My $.02. Thanks, John
On 2022/5/7 4:52, John Fastabend wrote: > Pu Lehui wrote: >> We found that 32-bit environment can not print bpf line info due >> to data inconsistency between jited_ksyms[0] and jited_linfo[0]. >> >> For example: >> jited_kyms[0] = 0xb800067c, jited_linfo[0] = 0xffffffffb800067c >> >> We know that both of them store bpf func address, but due to the >> different data extension operations when extended to u64, they may >> not be the same. We need to unify the data extension operations of >> them. >> >> Signed-off-by: Pu Lehui <pulehui@huawei.com> >> --- >> kernel/bpf/syscall.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c >> index e9e3e49c0eb7..18137ea5190d 100644 >> --- a/kernel/bpf/syscall.c >> +++ b/kernel/bpf/syscall.c >> @@ -3871,13 +3871,16 @@ static int bpf_prog_get_info_by_fd(struct file *file, >> info.nr_jited_line_info = 0; >> if (info.nr_jited_line_info && ulen) { >> if (bpf_dump_raw_ok(file->f_cred)) { >> + unsigned long jited_linfo_addr; >> __u64 __user *user_linfo; >> u32 i; >> >> user_linfo = u64_to_user_ptr(info.jited_line_info); >> ulen = min_t(u32, info.nr_jited_line_info, ulen); >> for (i = 0; i < ulen; i++) { >> - if (put_user((__u64)(long)prog->aux->jited_linfo[i], >> + jited_linfo_addr = (unsigned long) >> + prog->aux->jited_linfo[i]; >> + if (put_user((__u64) jited_linfo_addr, >> &user_linfo[i])) > > the logic is fine but i'm going to nitpick a bit this 4 lines is ugly > just make it slightly longer than 80chars or use a shoarter name? For > example, > > for (i = 0; i < ulen; i++) { > unsigned long l; > > l = (unsigned long) prog->aux->jited_linfo[i]; > if (put_user((__u64) l, &user_linfo[i])) > > is much nicer -- no reason to smash single assignment across multiple > lines. My $.02. > Okay, It sounds good. I will make change in next version. Thanks. > Thanks, > John > . >
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index e9e3e49c0eb7..18137ea5190d 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -3871,13 +3871,16 @@ static int bpf_prog_get_info_by_fd(struct file *file, info.nr_jited_line_info = 0; if (info.nr_jited_line_info && ulen) { if (bpf_dump_raw_ok(file->f_cred)) { + unsigned long jited_linfo_addr; __u64 __user *user_linfo; u32 i; user_linfo = u64_to_user_ptr(info.jited_line_info); ulen = min_t(u32, info.nr_jited_line_info, ulen); for (i = 0; i < ulen; i++) { - if (put_user((__u64)(long)prog->aux->jited_linfo[i], + jited_linfo_addr = (unsigned long) + prog->aux->jited_linfo[i]; + if (put_user((__u64) jited_linfo_addr, &user_linfo[i])) return -EFAULT; }
We found that 32-bit environment can not print bpf line info due to data inconsistency between jited_ksyms[0] and jited_linfo[0]. For example: jited_kyms[0] = 0xb800067c, jited_linfo[0] = 0xffffffffb800067c We know that both of them store bpf func address, but due to the different data extension operations when extended to u64, they may not be the same. We need to unify the data extension operations of them. Signed-off-by: Pu Lehui <pulehui@huawei.com> --- kernel/bpf/syscall.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)