| Message ID | 20220110165208.1826-1-jszhang@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | BPF |
| Headers | show |
| Series | [riscv-next] riscv: bpf: Fix eBPF's exception tables | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-next-PR | success | PR summary |
| bpf/vmtest-bpf-next | success | VM_Test |
| netdev/tree_selection | success | Not a local patch |
On Tue, Jan 11, 2022 at 12:52:08AM +0800, Jisheng Zhang wrote: > eBPF's exception tables needs to be modified to relative synchronously. > > Suggested-by: Tong Tiangen <tongtiangen@huawei.com> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > --- > arch/riscv/net/bpf_jit_comp64.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c > index 69bab7e28f91..44c97535bc15 100644 > --- a/arch/riscv/net/bpf_jit_comp64.c > +++ b/arch/riscv/net/bpf_jit_comp64.c > @@ -498,7 +498,7 @@ static int add_exception_handler(const struct bpf_insn *insn, > offset = pc - (long)&ex->insn; > if (WARN_ON_ONCE(offset >= 0 || offset < INT_MIN)) > return -ERANGE; > - ex->insn = pc; > + ex->insn = offset; Hi Palmer, Tong pointed out this issue but there was something wrong with my email forwarding address, so I didn't get his reply. Today, I searched on lore.kernel.org just found his reply, sorry for inconvenience. Thanks > > /* > * Since the extable follows the program, the fixup offset is always > -- > 2.34.1 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
Jisheng/Palmer, On Mon, 10 Jan 2022 at 18:05, Jisheng Zhang <jszhang@kernel.org> wrote: > > On Tue, Jan 11, 2022 at 12:52:08AM +0800, Jisheng Zhang wrote: > > eBPF's exception tables needs to be modified to relative synchronously. > > > > Suggested-by: Tong Tiangen <tongtiangen@huawei.com> > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> Nice catch, and apologies for the slow response. Acked-by: Björn Töpel <bjorn@kernel.org> > > --- > > arch/riscv/net/bpf_jit_comp64.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c > > index 69bab7e28f91..44c97535bc15 100644 > > --- a/arch/riscv/net/bpf_jit_comp64.c > > +++ b/arch/riscv/net/bpf_jit_comp64.c > > @@ -498,7 +498,7 @@ static int add_exception_handler(const struct bpf_insn *insn, > > offset = pc - (long)&ex->insn; > > if (WARN_ON_ONCE(offset >= 0 || offset < INT_MIN)) > > return -ERANGE; > > - ex->insn = pc; > > + ex->insn = offset; > > Hi Palmer, > > Tong pointed out this issue but there was something wrong with my email > forwarding address, so I didn't get his reply. Today, I searched on > lore.kernel.org just found his reply, sorry for inconvenience. > AFAIK, Jisheng's extable work is still in Palmer's for-next tree. Daniel/Alexei: This eBPF must follow commit 1f77ed9422cb ("riscv: switch to relative extable and other improvements"), which is in Palmer's tree. It cannot go via bpf-next. Palmer, please pull this to your for-next tree. Thanks, Björn
On 1/19/22 11:24 AM, Björn Töpel wrote: > On Mon, 10 Jan 2022 at 18:05, Jisheng Zhang <jszhang@kernel.org> wrote: >> On Tue, Jan 11, 2022 at 12:52:08AM +0800, Jisheng Zhang wrote: >>> eBPF's exception tables needs to be modified to relative synchronously. >>> >>> Suggested-by: Tong Tiangen <tongtiangen@huawei.com> >>> Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > Nice catch, and apologies for the slow response. > > Acked-by: Björn Töpel <bjorn@kernel.org> > >>> --- >>> arch/riscv/net/bpf_jit_comp64.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c >>> index 69bab7e28f91..44c97535bc15 100644 >>> --- a/arch/riscv/net/bpf_jit_comp64.c >>> +++ b/arch/riscv/net/bpf_jit_comp64.c >>> @@ -498,7 +498,7 @@ static int add_exception_handler(const struct bpf_insn *insn, >>> offset = pc - (long)&ex->insn; >>> if (WARN_ON_ONCE(offset >= 0 || offset < INT_MIN)) >>> return -ERANGE; >>> - ex->insn = pc; >>> + ex->insn = offset; >> >> Hi Palmer, >> >> Tong pointed out this issue but there was something wrong with my email >> forwarding address, so I didn't get his reply. Today, I searched on >> lore.kernel.org just found his reply, sorry for inconvenience. > > AFAIK, Jisheng's extable work is still in Palmer's for-next tree. > > Daniel/Alexei: This eBPF must follow commit 1f77ed9422cb ("riscv: > switch to relative extable and other improvements"), which is in > Palmer's tree. It cannot go via bpf-next. Thanks for letting us know, then lets route this fix via Palmer. Maybe he could also add Fixes tags when applying, so stable can pick it up later on. Cheers, Daniel
On Wed, 19 Jan 2022 at 16:42, Daniel Borkmann <daniel@iogearbox.net> wrote: > [...] > > AFAIK, Jisheng's extable work is still in Palmer's for-next tree. > > > > Daniel/Alexei: This eBPF must follow commit 1f77ed9422cb ("riscv: > > switch to relative extable and other improvements"), which is in > > Palmer's tree. It cannot go via bpf-next. > > Thanks for letting us know, then lets route this fix via Palmer. Maybe he could > also add Fixes tags when applying, so stable can pick it up later on. > It shouldn't have a fixes-tag, since it's a new feature for RV. This was adapting to that new feature. It hasn't made it upstream yet (I hope!). Cheers, Björn
On Wed, 19 Jan 2022 07:59:40 PST (-0800), Bjorn Topel wrote: > On Wed, 19 Jan 2022 at 16:42, Daniel Borkmann <daniel@iogearbox.net> wrote: >> > [...] >> > AFAIK, Jisheng's extable work is still in Palmer's for-next tree. >> > >> > Daniel/Alexei: This eBPF must follow commit 1f77ed9422cb ("riscv: >> > switch to relative extable and other improvements"), which is in >> > Palmer's tree. It cannot go via bpf-next. >> >> Thanks for letting us know, then lets route this fix via Palmer. Maybe he could >> also add Fixes tags when applying, so stable can pick it up later on. >> > > It shouldn't have a fixes-tag, since it's a new feature for RV. This > was adapting to that new feature. It hasn't made it upstream yet (I > hope!). That was actually just merged this morning into Linus' tree. I'm still happy to take the fix via my tree, but you're welcome to take it via a BPF tree as well. I'm juggling some other patches right now, just LMK what works on your end. IMO it should have a fixes tag: it's a bit of a grey area, but one something's in I generally try and put those tags on it. That way folks who try and backport features at least have a shot at finding the fix (or at least, finding the fix without chasing around the bug ;)). I also tried poking you guys on the BPF Slack, but I don't really use it and I'm not sure if anyone else does either.
On Wed, 19 Jan 2022 10:04:24 PST (-0800), Palmer Dabbelt wrote: > On Wed, 19 Jan 2022 07:59:40 PST (-0800), Bjorn Topel wrote: >> On Wed, 19 Jan 2022 at 16:42, Daniel Borkmann <daniel@iogearbox.net> wrote: >>> >> [...] >>> > AFAIK, Jisheng's extable work is still in Palmer's for-next tree. >>> > >>> > Daniel/Alexei: This eBPF must follow commit 1f77ed9422cb ("riscv: >>> > switch to relative extable and other improvements"), which is in >>> > Palmer's tree. It cannot go via bpf-next. >>> >>> Thanks for letting us know, then lets route this fix via Palmer. Maybe he could >>> also add Fixes tags when applying, so stable can pick it up later on. >>> >> >> It shouldn't have a fixes-tag, since it's a new feature for RV. This >> was adapting to that new feature. It hasn't made it upstream yet (I >> hope!). > > That was actually just merged this morning into Linus' tree. I'm still > happy to take the fix via my tree, but you're welcome to take it via a > BPF tree as well. I'm juggling some other patches right now, just LMK > what works on your end. > > IMO it should have a fixes tag: it's a bit of a grey area, but one > something's in I generally try and put those tags on it. That way folks > who try and backport features at least have a shot at finding the fix > (or at least, finding the fix without chasing around the bug ;)). > > I also tried poking you guys on the BPF Slack, but I don't really use it > and I'm not sure if anyone else does either. As per the slack discussion with Daniel, I've put this into the RISC-V for-next tree. Thanks!
diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c index 69bab7e28f91..44c97535bc15 100644 --- a/arch/riscv/net/bpf_jit_comp64.c +++ b/arch/riscv/net/bpf_jit_comp64.c @@ -498,7 +498,7 @@ static int add_exception_handler(const struct bpf_insn *insn, offset = pc - (long)&ex->insn; if (WARN_ON_ONCE(offset >= 0 || offset < INT_MIN)) return -ERANGE; - ex->insn = pc; + ex->insn = offset; /* * Since the extable follows the program, the fixup offset is always
eBPF's exception tables needs to be modified to relative synchronously. Suggested-by: Tong Tiangen <tongtiangen@huawei.com> Signed-off-by: Jisheng Zhang <jszhang@kernel.org> --- arch/riscv/net/bpf_jit_comp64.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)