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

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

[Jisheng Zhang]

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;
 
 	/*
 	 * Since the extable follows the program, the fixup offset is always
-- 
2.34.1

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

[Jisheng Zhang]

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

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

[Björn Töpel]

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

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

[Daniel Borkmann]

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

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

[Björn Töpel]

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

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

[Palmer Dabbelt]

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.

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

[Palmer Dabbelt]

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!