2022-01-10 16:59:45

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH riscv-next] riscv: bpf: Fix eBPF's exception tables

eBPF's exception tables needs to be modified to relative synchronously.

Suggested-by: Tong Tiangen <[email protected]>
Signed-off-by: Jisheng Zhang <[email protected]>
---
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;

/*
* Since the extable follows the program, the fixup offset is always
--
2.34.1



2022-01-10 17:05:24

by Jisheng Zhang

[permalink] [raw]
Subject: Re: [PATCH riscv-next] riscv: bpf: Fix eBPF's exception tables

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 <[email protected]>
> Signed-off-by: Jisheng Zhang <[email protected]>
> ---
> 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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2022-01-21 19:08:27

by Björn Töpel

[permalink] [raw]
Subject: Re: [PATCH riscv-next] riscv: bpf: Fix eBPF's exception tables

Jisheng/Palmer,

On Mon, 10 Jan 2022 at 18:05, Jisheng Zhang <[email protected]> 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 <[email protected]>
> > Signed-off-by: Jisheng Zhang <[email protected]>

Nice catch, and apologies for the slow response.

Acked-by: Björn Töpel <[email protected]>

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

2022-01-21 19:40:02

by Daniel Borkmann

[permalink] [raw]
Subject: Re: [PATCH riscv-next] riscv: bpf: Fix eBPF's exception tables

On 1/19/22 11:24 AM, Björn Töpel wrote:
> On Mon, 10 Jan 2022 at 18:05, Jisheng Zhang <[email protected]> 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 <[email protected]>
>>> Signed-off-by: Jisheng Zhang <[email protected]>
>
> Nice catch, and apologies for the slow response.
>
> Acked-by: Björn Töpel <[email protected]>
>
>>> ---
>>> 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

2022-01-21 19:44:29

by Björn Töpel

[permalink] [raw]
Subject: Re: [PATCH riscv-next] riscv: bpf: Fix eBPF's exception tables

On Wed, 19 Jan 2022 at 16:42, Daniel Borkmann <[email protected]> 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

2022-01-21 19:54:49

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH riscv-next] riscv: bpf: Fix eBPF's exception tables

On Wed, 19 Jan 2022 07:59:40 PST (-0800), Bjorn Topel wrote:
> On Wed, 19 Jan 2022 at 16:42, Daniel Borkmann <[email protected]> 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.

2022-01-21 20:06:21

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH riscv-next] riscv: bpf: Fix eBPF's exception tables

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 <[email protected]> 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!