bpf: Add mptcp_sched_ops CFI

[PATCH mptcp-next] bpf: Add mptcp_sched_ops CFI

Posted by Geliang Tang 2 years, 1 month ago

'''
The cfi_stubs pointer is added in the commit 2cd3e3772e41 ("x86/cfi,bpf:
Fix bpf_struct_ops CFI"), this patch initializes it as dummy stubs
__bpf_mptcp_sched_ops.
'''

Note:

BPF tests on export branch fail with this error:

 BUG: kernel NULL pointer dereference, address: 0000000000000000
 #PF: supervisor read access in kernel mode
 #PF: error_code(0x0000) - not-present page
 PGD 0 P4D 0
 Oops: 0000 [#1] PREEMPT SMP PTI
 CPU: 3 PID: 1512 Comm: test_progs Tainted: G        W IOE      6.7.0-rc5 #37
 Hardware name: LENOVO 4180PG2/4180PG2, BIOS 83ET76WW (1.46 ) 07/05/2013
 RIP: 0010:bpf_struct_ops_map_update_elem+0x4ee/0x610

This patch fixes this error. It can be applied after the commit
"bpf: Add bpf_mptcp_sched_ops"

Signed-off-by: Geliang Tang <geliang.tang@linux.dev>
---
 net/mptcp/bpf.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
index b92c9014ac2c..fb67768aa07d 100644
--- a/net/mptcp/bpf.c
+++ b/net/mptcp/bpf.c
@@ -147,6 +147,19 @@ static int bpf_mptcp_sched_init(struct btf *btf)
 	return 0;
 }
 
+static void __bpf_mptcp_sched_init(struct mptcp_sock *msk)
+{
+}
+
+static void __bpf_mptcp_sched_release(struct mptcp_sock *msk)
+{
+}
+
+static struct mptcp_sched_ops __bpf_mptcp_sched_ops = {
+	.init		= __bpf_mptcp_sched_init,
+	.release	= __bpf_mptcp_sched_release,
+};
+
 struct bpf_struct_ops bpf_mptcp_sched_ops = {
 	.verifier_ops	= &bpf_mptcp_sched_verifier_ops,
 	.reg		= bpf_mptcp_sched_reg,
@@ -155,6 +168,7 @@ struct bpf_struct_ops bpf_mptcp_sched_ops = {
 	.init_member	= bpf_mptcp_sched_init_member,
 	.init		= bpf_mptcp_sched_init,
 	.name		= "mptcp_sched_ops",
+	.cfi_stubs	= &__bpf_mptcp_sched_ops,
 };
 #endif /* CONFIG_BPF_JIT */
 
-- 
2.39.2

Re: [PATCH mptcp-next] bpf: Add mptcp_sched_ops CFI

Posted by Matthieu Baerts 2 years, 1 month ago

Hi Geliang,

On 06/01/2024 09:40, Geliang Tang wrote:
> '''
> The cfi_stubs pointer is added in the commit 2cd3e3772e41 ("x86/cfi,bpf:
> Fix bpf_struct_ops CFI"), this patch initializes it as dummy stubs
> __bpf_mptcp_sched_ops.
> '''
> 
> Note:
> 
> BPF tests on export branch fail with this error:
> 
>  BUG: kernel NULL pointer dereference, address: 0000000000000000
>  #PF: supervisor read access in kernel mode
>  #PF: error_code(0x0000) - not-present page
>  PGD 0 P4D 0
>  Oops: 0000 [#1] PREEMPT SMP PTI
>  CPU: 3 PID: 1512 Comm: test_progs Tainted: G        W IOE      6.7.0-rc5 #37
>  Hardware name: LENOVO 4180PG2/4180PG2, BIOS 83ET76WW (1.46 ) 07/05/2013
>  RIP: 0010:bpf_struct_ops_map_update_elem+0x4ee/0x610
> 
> This patch fixes this error. It can be applied after the commit
> "bpf: Add bpf_mptcp_sched_ops"

Thank you for having checked and fixed that!

Is it OK for you if I squash it in "bpf: Add bpf_mptcp_sched_ops" commit?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-next] bpf: Add mptcp_sched_ops CFI

Posted by Geliang Tang 2 years, 1 month ago

Hi Matt,

On Mon, 2024-01-08 at 11:14 +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 06/01/2024 09:40, Geliang Tang wrote:
> > '''
> > The cfi_stubs pointer is added in the commit 2cd3e3772e41
> > ("x86/cfi,bpf:
> > Fix bpf_struct_ops CFI"), this patch initializes it as dummy stubs
> > __bpf_mptcp_sched_ops.
> > '''
> > 
> > Note:
> > 
> > BPF tests on export branch fail with this error:
> > 
> >  BUG: kernel NULL pointer dereference, address: 0000000000000000
> >  #PF: supervisor read access in kernel mode
> >  #PF: error_code(0x0000) - not-present page
> >  PGD 0 P4D 0
> >  Oops: 0000 [#1] PREEMPT SMP PTI
> >  CPU: 3 PID: 1512 Comm: test_progs Tainted: G        W IOE     
> > 6.7.0-rc5 #37
> >  Hardware name: LENOVO 4180PG2/4180PG2, BIOS 83ET76WW (1.46 )
> > 07/05/2013
> >  RIP: 0010:bpf_struct_ops_map_update_elem+0x4ee/0x610
> > 
> > This patch fixes this error. It can be applied after the commit
> > "bpf: Add bpf_mptcp_sched_ops"
> 
> Thank you for having checked and fixed that!
> 
> Is it OK for you if I squash it in "bpf: Add bpf_mptcp_sched_ops"
> commit?

Yes, it's better to squash it. Please do this when merging it.

I'm not sure whether "get_subflow" should be initialized here too. If
it should be, I'll send another squash-to patch to fix this.

Thanks,
-Geliang

> 
> Cheers,
> Matt

Re: [PATCH mptcp-next] bpf: Add mptcp_sched_ops CFI

Posted by Matthieu Baerts 2 years, 1 month ago

Hi Geliang,

On 08/01/2024 13:37, Geliang Tang wrote:
> Hi Matt,
> 
> On Mon, 2024-01-08 at 11:14 +0100, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> On 06/01/2024 09:40, Geliang Tang wrote:
>>> '''
>>> The cfi_stubs pointer is added in the commit 2cd3e3772e41
>>> ("x86/cfi,bpf:
>>> Fix bpf_struct_ops CFI"), this patch initializes it as dummy stubs
>>> __bpf_mptcp_sched_ops.
>>> '''
>>>
>>> Note:
>>>
>>> BPF tests on export branch fail with this error:
>>>
>>>  BUG: kernel NULL pointer dereference, address: 0000000000000000
>>>  #PF: supervisor read access in kernel mode
>>>  #PF: error_code(0x0000) - not-present page
>>>  PGD 0 P4D 0
>>>  Oops: 0000 [#1] PREEMPT SMP PTI
>>>  CPU: 3 PID: 1512 Comm: test_progs Tainted: G        W IOE     
>>> 6.7.0-rc5 #37
>>>  Hardware name: LENOVO 4180PG2/4180PG2, BIOS 83ET76WW (1.46 )
>>> 07/05/2013
>>>  RIP: 0010:bpf_struct_ops_map_update_elem+0x4ee/0x610
>>>
>>> This patch fixes this error. It can be applied after the commit
>>> "bpf: Add bpf_mptcp_sched_ops"
>>
>> Thank you for having checked and fixed that!
>>
>> Is it OK for you if I squash it in "bpf: Add bpf_mptcp_sched_ops"
>> commit?
> 
> Yes, it's better to squash it. Please do this when merging it.
> 
> I'm not sure whether "get_subflow" should be initialized here too. If
> it should be, I'll send another squash-to patch to fix this.

Good point, I guess it should. When I look at bpf_tcp_ca.c, it looks
like they implemented all function pointers. Except get_info(), but this
one is marked as unsupported.

I just added it when applying your patch. Please tell me if I did it wrong.

While at it, I also updated your email address (I really thought I did
it before, but apparently not...)

New patches for t/upstream:
- 810cc99b7dd6: bpf: Add mptcp_sched_ops CFI
- 197ce89d9597: mptcp: bpf: add get_subflow for CFI
- Results: ca5de126f347..10ca1d146a6a (export)

- ca17d989b252: tg:msg: update Geliang's email address
- 9553337100f5: tg:msg: update Geliang's email address
- 404ca606684e: tg:msg: update Geliang's email address
- 75f8cc7247d5: tg:msg: update Geliang's email address
- d86fc89e4e11: tg:msg: update Geliang's email address
- e8708c02eace: tg:msg: update Geliang's email address
- 039488fe8e2e: tg:msg: update Geliang's email address
- 9bbd8dbced1d: tg:msg: update Geliang's email address
- 8f03f584b792: tg:msg: update Geliang's email address
- b500d1b51c5b: tg:msg: update Geliang's email address
- 4e489b19e9f0: tg:msg: update Geliang's email address
- e2029e1cfb8b: tg:msg: update Geliang's email address
- 134f88acf139: tg:msg: update Geliang's email address
- b708aa7547e1: tg:msg: update Geliang's email address
- a814d467f027: tg:msg: update Geliang's email address
- 2b1c988725e8: tg:msg: update Geliang's email address
- Results: 10ca1d146a6a..bd5be2c4e44b (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20240108T172308

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.