BPF List
 help / color / mirror / Atom feed
* [PATCH bpf] bpf: Fix null pointer dereference in resolve_prog_type() for BPF_PROG_TYPE_EXT
@ 2024-06-20  6:07 Tengda Wu
  2024-06-20  6:46 ` Leon Hwang
  2024-06-20 10:28 ` Leon Hwang
  0 siblings, 2 replies; 5+ messages in thread
From: Tengda Wu @ 2024-06-20  6:07 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa

When loading a EXT program without specifying `attr->attach_prog_fd`,
the `prog->aux->dst_prog` will be null. At this time, calling
resolve_prog_type() anywhere will result in a null pointer dereference.

Example stack trace:

[    8.107863] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000004
[    8.108262] Mem abort info:
[    8.108384]   ESR = 0x0000000096000004
[    8.108547]   EC = 0x25: DABT (current EL), IL = 32 bits
[    8.108722]   SET = 0, FnV = 0
[    8.108827]   EA = 0, S1PTW = 0
[    8.108939]   FSC = 0x04: level 0 translation fault
[    8.109102] Data abort info:
[    8.109203]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[    8.109399]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[    8.109614]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[    8.109836] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000101354000
[    8.110011] [0000000000000004] pgd=0000000000000000, p4d=0000000000000000
[    8.112624] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
[    8.112783] Modules linked in:
[    8.113120] CPU: 0 PID: 99 Comm: may_access_dire Not tainted 6.10.0-rc3-next-20240613-dirty #1
[    8.113230] Hardware name: linux,dummy-virt (DT)
[    8.113390] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    8.113429] pc : may_access_direct_pkt_data+0x24/0xa0
[    8.113746] lr : add_subprog_and_kfunc+0x634/0x8e8
[    8.113798] sp : ffff80008283b9f0
[    8.113813] x29: ffff80008283b9f0 x28: ffff800082795048 x27: 0000000000000001
[    8.113881] x26: ffff0000c0bb2600 x25: 0000000000000000 x24: 0000000000000000
[    8.113897] x23: ffff0000c1134000 x22: 000000000001864f x21: ffff0000c1138000
[    8.113912] x20: 0000000000000001 x19: ffff0000c12b8000 x18: ffffffffffffffff
[    8.113929] x17: 0000000000000000 x16: 0000000000000000 x15: 0720072007200720
[    8.113944] x14: 0720072007200720 x13: 0720072007200720 x12: 0720072007200720
[    8.113958] x11: 0720072007200720 x10: 0000000000f9fca4 x9 : ffff80008021f4e4
[    8.113991] x8 : 0101010101010101 x7 : 746f72705f6d656d x6 : 000000001e0e0f5f
[    8.114006] x5 : 000000000001864f x4 : ffff0000c12b8000 x3 : 000000000000001c
[    8.114020] x2 : 0000000000000002 x1 : 0000000000000000 x0 : 0000000000000000
[    8.114126] Call trace:
[    8.114159]  may_access_direct_pkt_data+0x24/0xa0
[    8.114202]  bpf_check+0x3bc/0x28c0
[    8.114214]  bpf_prog_load+0x658/0xa58
[    8.114227]  __sys_bpf+0xc50/0x2250
[    8.114240]  __arm64_sys_bpf+0x28/0x40
[    8.114254]  invoke_syscall.constprop.0+0x54/0xf0
[    8.114273]  do_el0_svc+0x4c/0xd8
[    8.114289]  el0_svc+0x3c/0x140
[    8.114305]  el0t_64_sync_handler+0x134/0x150
[    8.114331]  el0t_64_sync+0x168/0x170
[    8.114477] Code: 7100707f 54000081 f9401c00 f9403800 (b9400403)
[    8.118672] ---[ end trace 0000000000000000 ]---

Fix this by adding dst_prog non-empty check in BPF_PROG_TYPE_EXT case
when calling bpf_prog_load().

Fixes: 4a9c7bbe2ed4 ("bpf: Resolve to prog->aux->dst_prog->type only for BPF_PROG_TYPE_EXT")
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
Cc: stable@vger.kernel.org # v5.18+
---
 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 f45ed6adc092..4490f8ccf006 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2632,9 +2632,12 @@ bpf_prog_load_check_attach(enum bpf_prog_type prog_type,
 			return 0;
 		return -EINVAL;
 	case BPF_PROG_TYPE_SYSCALL:
-	case BPF_PROG_TYPE_EXT:
 		if (expected_attach_type)
 			return -EINVAL;
+		return 0;
+	case BPF_PROG_TYPE_EXT:
+		if (expected_attach_type || !dst_prog)
+			return -EINVAL;
 		fallthrough;
 	default:
 		return 0;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf] bpf: Fix null pointer dereference in resolve_prog_type() for BPF_PROG_TYPE_EXT
  2024-06-20  6:07 [PATCH bpf] bpf: Fix null pointer dereference in resolve_prog_type() for BPF_PROG_TYPE_EXT Tengda Wu
@ 2024-06-20  6:46 ` Leon Hwang
  2024-06-20  8:54   ` Tengda Wu
  2024-06-20 10:28 ` Leon Hwang
  1 sibling, 1 reply; 5+ messages in thread
From: Leon Hwang @ 2024-06-20  6:46 UTC (permalink / raw)
  To: Tengda Wu, bpf
  Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa



On 20/6/24 14:07, Tengda Wu wrote:
> When loading a EXT program without specifying `attr->attach_prog_fd`,
> the `prog->aux->dst_prog` will be null. At this time, calling
> resolve_prog_type() anywhere will result in a null pointer dereference.

Interesting, same NULL pointer dereference causes another issue[0].

As for my case, when resolve_prog_type(), it has to use
prog->aux->saved_dst_prog_type instead of prog->aux->dst_prog->type for
EXT program, in order to avoid NULL pointer dereference.

[0] https://lore.kernel.org/bpf/20240602122421.50892-2-hffilwlqm@gmail.com/

Thanks,
Leon

> 
> Example stack trace:
> 
> [    8.107863] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000004
> [    8.108262] Mem abort info:
> [    8.108384]   ESR = 0x0000000096000004
> [    8.108547]   EC = 0x25: DABT (current EL), IL = 32 bits
> [    8.108722]   SET = 0, FnV = 0
> [    8.108827]   EA = 0, S1PTW = 0
> [    8.108939]   FSC = 0x04: level 0 translation fault
> [    8.109102] Data abort info:
> [    8.109203]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> [    8.109399]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> [    8.109614]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [    8.109836] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000101354000
> [    8.110011] [0000000000000004] pgd=0000000000000000, p4d=0000000000000000
> [    8.112624] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
> [    8.112783] Modules linked in:
> [    8.113120] CPU: 0 PID: 99 Comm: may_access_dire Not tainted 6.10.0-rc3-next-20240613-dirty #1
> [    8.113230] Hardware name: linux,dummy-virt (DT)
> [    8.113390] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [    8.113429] pc : may_access_direct_pkt_data+0x24/0xa0
> [    8.113746] lr : add_subprog_and_kfunc+0x634/0x8e8
> [    8.113798] sp : ffff80008283b9f0
> [    8.113813] x29: ffff80008283b9f0 x28: ffff800082795048 x27: 0000000000000001
> [    8.113881] x26: ffff0000c0bb2600 x25: 0000000000000000 x24: 0000000000000000
> [    8.113897] x23: ffff0000c1134000 x22: 000000000001864f x21: ffff0000c1138000
> [    8.113912] x20: 0000000000000001 x19: ffff0000c12b8000 x18: ffffffffffffffff
> [    8.113929] x17: 0000000000000000 x16: 0000000000000000 x15: 0720072007200720
> [    8.113944] x14: 0720072007200720 x13: 0720072007200720 x12: 0720072007200720
> [    8.113958] x11: 0720072007200720 x10: 0000000000f9fca4 x9 : ffff80008021f4e4
> [    8.113991] x8 : 0101010101010101 x7 : 746f72705f6d656d x6 : 000000001e0e0f5f
> [    8.114006] x5 : 000000000001864f x4 : ffff0000c12b8000 x3 : 000000000000001c
> [    8.114020] x2 : 0000000000000002 x1 : 0000000000000000 x0 : 0000000000000000
> [    8.114126] Call trace:
> [    8.114159]  may_access_direct_pkt_data+0x24/0xa0
> [    8.114202]  bpf_check+0x3bc/0x28c0
> [    8.114214]  bpf_prog_load+0x658/0xa58
> [    8.114227]  __sys_bpf+0xc50/0x2250
> [    8.114240]  __arm64_sys_bpf+0x28/0x40
> [    8.114254]  invoke_syscall.constprop.0+0x54/0xf0
> [    8.114273]  do_el0_svc+0x4c/0xd8
> [    8.114289]  el0_svc+0x3c/0x140
> [    8.114305]  el0t_64_sync_handler+0x134/0x150
> [    8.114331]  el0t_64_sync+0x168/0x170
> [    8.114477] Code: 7100707f 54000081 f9401c00 f9403800 (b9400403)
> [    8.118672] ---[ end trace 0000000000000000 ]---
> 
> Fix this by adding dst_prog non-empty check in BPF_PROG_TYPE_EXT case
> when calling bpf_prog_load().
> 
> Fixes: 4a9c7bbe2ed4 ("bpf: Resolve to prog->aux->dst_prog->type only for BPF_PROG_TYPE_EXT")
> Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
> Cc: stable@vger.kernel.org # v5.18+
> ---
>  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 f45ed6adc092..4490f8ccf006 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2632,9 +2632,12 @@ bpf_prog_load_check_attach(enum bpf_prog_type prog_type,
>  			return 0;
>  		return -EINVAL;
>  	case BPF_PROG_TYPE_SYSCALL:
> -	case BPF_PROG_TYPE_EXT:
>  		if (expected_attach_type)
>  			return -EINVAL;
> +		return 0;
> +	case BPF_PROG_TYPE_EXT:
> +		if (expected_attach_type || !dst_prog)
> +			return -EINVAL;
>  		fallthrough;
>  	default:
>  		return 0;

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf] bpf: Fix null pointer dereference in resolve_prog_type() for BPF_PROG_TYPE_EXT
  2024-06-20  6:46 ` Leon Hwang
@ 2024-06-20  8:54   ` Tengda Wu
  2024-06-20 10:27     ` Jiri Olsa
  0 siblings, 1 reply; 5+ messages in thread
From: Tengda Wu @ 2024-06-20  8:54 UTC (permalink / raw)
  To: Leon Hwang, bpf
  Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa



On 2024/6/20 14:46, Leon Hwang wrote:
> 
> 
> On 20/6/24 14:07, Tengda Wu wrote:
>> When loading a EXT program without specifying `attr->attach_prog_fd`,
>> the `prog->aux->dst_prog` will be null. At this time, calling
>> resolve_prog_type() anywhere will result in a null pointer dereference.
> 
> Interesting, same NULL pointer dereference causes another issue[0].
> 
> As for my case, when resolve_prog_type(), it has to use
> prog->aux->saved_dst_prog_type instead of prog->aux->dst_prog->type for
> EXT program, in order to avoid NULL pointer dereference.
> 
> [0] https://lore.kernel.org/bpf/20240602122421.50892-2-hffilwlqm@gmail.com/
> 
> Thanks,
> Leon
>This looks good, but unfortunately, there is still a problem with using 
`prog->aux->saved_dst_prog_type` to resolve prog type, because its value still 
comes from `prog->aux->dst_prog`in check_attach_btf_id(). 

Additionally, resolve_prog_type() not always be used after check_attach_btf_id().
The following example stack trace proves the existence of this situation. It 
shows that NULL pointer dereference occurs in add_subprog_and_kfunc(), which
check_attach_btf_id() has not yet reached. 

So it may be more effective to check and avoid dst_prog empty when EXT program loads.

>>
>> Example stack trace:
>>
>> [    8.107863] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000004
>> [    8.108262] Mem abort info:
>> [    8.108384]   ESR = 0x0000000096000004
>> [    8.108547]   EC = 0x25: DABT (current EL), IL = 32 bits
>> [    8.108722]   SET = 0, FnV = 0
>> [    8.108827]   EA = 0, S1PTW = 0
>> [    8.108939]   FSC = 0x04: level 0 translation fault
>> [    8.109102] Data abort info:
>> [    8.109203]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
>> [    8.109399]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
>> [    8.109614]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
>> [    8.109836] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000101354000
>> [    8.110011] [0000000000000004] pgd=0000000000000000, p4d=0000000000000000
>> [    8.112624] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
>> [    8.112783] Modules linked in:
>> [    8.113120] CPU: 0 PID: 99 Comm: may_access_dire Not tainted 6.10.0-rc3-next-20240613-dirty #1
>> [    8.113230] Hardware name: linux,dummy-virt (DT)
>> [    8.113390] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> [    8.113429] pc : may_access_direct_pkt_data+0x24/0xa0
>> [    8.113746] lr : add_subprog_and_kfunc+0x634/0x8e8
>> [    8.113798] sp : ffff80008283b9f0
>> [    8.113813] x29: ffff80008283b9f0 x28: ffff800082795048 x27: 0000000000000001
>> [    8.113881] x26: ffff0000c0bb2600 x25: 0000000000000000 x24: 0000000000000000
>> [    8.113897] x23: ffff0000c1134000 x22: 000000000001864f x21: ffff0000c1138000
>> [    8.113912] x20: 0000000000000001 x19: ffff0000c12b8000 x18: ffffffffffffffff
>> [    8.113929] x17: 0000000000000000 x16: 0000000000000000 x15: 0720072007200720
>> [    8.113944] x14: 0720072007200720 x13: 0720072007200720 x12: 0720072007200720
>> [    8.113958] x11: 0720072007200720 x10: 0000000000f9fca4 x9 : ffff80008021f4e4
>> [    8.113991] x8 : 0101010101010101 x7 : 746f72705f6d656d x6 : 000000001e0e0f5f
>> [    8.114006] x5 : 000000000001864f x4 : ffff0000c12b8000 x3 : 000000000000001c
>> [    8.114020] x2 : 0000000000000002 x1 : 0000000000000000 x0 : 0000000000000000
>> [    8.114126] Call trace:
>> [    8.114159]  may_access_direct_pkt_data+0x24/0xa0
>> [    8.114202]  bpf_check+0x3bc/0x28c0
>> [    8.114214]  bpf_prog_load+0x658/0xa58
>> [    8.114227]  __sys_bpf+0xc50/0x2250
>> [    8.114240]  __arm64_sys_bpf+0x28/0x40
>> [    8.114254]  invoke_syscall.constprop.0+0x54/0xf0
>> [    8.114273]  do_el0_svc+0x4c/0xd8
>> [    8.114289]  el0_svc+0x3c/0x140
>> [    8.114305]  el0t_64_sync_handler+0x134/0x150
>> [    8.114331]  el0t_64_sync+0x168/0x170
>> [    8.114477] Code: 7100707f 54000081 f9401c00 f9403800 (b9400403)
>> [    8.118672] ---[ end trace 0000000000000000 ]---
>>
>> Fix this by adding dst_prog non-empty check in BPF_PROG_TYPE_EXT case
>> when calling bpf_prog_load().
>>
>> Fixes: 4a9c7bbe2ed4 ("bpf: Resolve to prog->aux->dst_prog->type only for BPF_PROG_TYPE_EXT")
>> Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
>> Cc: stable@vger.kernel.org # v5.18+
>> ---
>>  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 f45ed6adc092..4490f8ccf006 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -2632,9 +2632,12 @@ bpf_prog_load_check_attach(enum bpf_prog_type prog_type,
>>  			return 0;
>>  		return -EINVAL;
>>  	case BPF_PROG_TYPE_SYSCALL:
>> -	case BPF_PROG_TYPE_EXT:
>>  		if (expected_attach_type)
>>  			return -EINVAL;
>> +		return 0;
>> +	case BPF_PROG_TYPE_EXT:
>> +		if (expected_attach_type || !dst_prog)
>> +			return -EINVAL;
>>  		fallthrough;
>>  	default:
>>  		return 0;


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf] bpf: Fix null pointer dereference in resolve_prog_type() for BPF_PROG_TYPE_EXT
  2024-06-20  8:54   ` Tengda Wu
@ 2024-06-20 10:27     ` Jiri Olsa
  0 siblings, 0 replies; 5+ messages in thread
From: Jiri Olsa @ 2024-06-20 10:27 UTC (permalink / raw)
  To: Tengda Wu
  Cc: Leon Hwang, bpf, Alexei Starovoitov, Daniel Borkmann,
	John Fastabend, Andrii Nakryiko, Martin KaFai Lau,
	Eduard Zingerman, Song Liu, Yonghong Song, KP Singh,
	Stanislav Fomichev, Hao Luo

On Thu, Jun 20, 2024 at 04:54:51PM +0800, Tengda Wu wrote:
> 
> 
> On 2024/6/20 14:46, Leon Hwang wrote:
> > 
> > 
> > On 20/6/24 14:07, Tengda Wu wrote:
> >> When loading a EXT program without specifying `attr->attach_prog_fd`,
> >> the `prog->aux->dst_prog` will be null. At this time, calling
> >> resolve_prog_type() anywhere will result in a null pointer dereference.
> > 
> > Interesting, same NULL pointer dereference causes another issue[0].
> > 
> > As for my case, when resolve_prog_type(), it has to use
> > prog->aux->saved_dst_prog_type instead of prog->aux->dst_prog->type for
> > EXT program, in order to avoid NULL pointer dereference.
> > 
> > [0] https://lore.kernel.org/bpf/20240602122421.50892-2-hffilwlqm@gmail.com/
> > 
> > Thanks,
> > Leon
> >This looks good, but unfortunately, there is still a problem with using 
> `prog->aux->saved_dst_prog_type` to resolve prog type, because its value still 
> comes from `prog->aux->dst_prog`in check_attach_btf_id(). 
> 
> Additionally, resolve_prog_type() not always be used after check_attach_btf_id().
> The following example stack trace proves the existence of this situation. It 
> shows that NULL pointer dereference occurs in add_subprog_and_kfunc(), which
> check_attach_btf_id() has not yet reached. 
> 
> So it may be more effective to check and avoid dst_prog empty when EXT program loads.

also please note it's breaking test_libbpf_probe_prog_types test

  test_libbpf_probe_prog_types:FAIL:BPF_PROG_TYPE_EXT unexpected BPF_PROG_TYPE_EXT: actual 0 != expected 1

because the attach_prog_fd wasn't needed to load EXT program before,
but I guess the following attach would fail.. so it's likely ok

jirka

> 
> >>
> >> Example stack trace:
> >>
> >> [    8.107863] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000004
> >> [    8.108262] Mem abort info:
> >> [    8.108384]   ESR = 0x0000000096000004
> >> [    8.108547]   EC = 0x25: DABT (current EL), IL = 32 bits
> >> [    8.108722]   SET = 0, FnV = 0
> >> [    8.108827]   EA = 0, S1PTW = 0
> >> [    8.108939]   FSC = 0x04: level 0 translation fault
> >> [    8.109102] Data abort info:
> >> [    8.109203]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> >> [    8.109399]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> >> [    8.109614]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> >> [    8.109836] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000101354000
> >> [    8.110011] [0000000000000004] pgd=0000000000000000, p4d=0000000000000000
> >> [    8.112624] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
> >> [    8.112783] Modules linked in:
> >> [    8.113120] CPU: 0 PID: 99 Comm: may_access_dire Not tainted 6.10.0-rc3-next-20240613-dirty #1
> >> [    8.113230] Hardware name: linux,dummy-virt (DT)
> >> [    8.113390] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> >> [    8.113429] pc : may_access_direct_pkt_data+0x24/0xa0
> >> [    8.113746] lr : add_subprog_and_kfunc+0x634/0x8e8
> >> [    8.113798] sp : ffff80008283b9f0
> >> [    8.113813] x29: ffff80008283b9f0 x28: ffff800082795048 x27: 0000000000000001
> >> [    8.113881] x26: ffff0000c0bb2600 x25: 0000000000000000 x24: 0000000000000000
> >> [    8.113897] x23: ffff0000c1134000 x22: 000000000001864f x21: ffff0000c1138000
> >> [    8.113912] x20: 0000000000000001 x19: ffff0000c12b8000 x18: ffffffffffffffff
> >> [    8.113929] x17: 0000000000000000 x16: 0000000000000000 x15: 0720072007200720
> >> [    8.113944] x14: 0720072007200720 x13: 0720072007200720 x12: 0720072007200720
> >> [    8.113958] x11: 0720072007200720 x10: 0000000000f9fca4 x9 : ffff80008021f4e4
> >> [    8.113991] x8 : 0101010101010101 x7 : 746f72705f6d656d x6 : 000000001e0e0f5f
> >> [    8.114006] x5 : 000000000001864f x4 : ffff0000c12b8000 x3 : 000000000000001c
> >> [    8.114020] x2 : 0000000000000002 x1 : 0000000000000000 x0 : 0000000000000000
> >> [    8.114126] Call trace:
> >> [    8.114159]  may_access_direct_pkt_data+0x24/0xa0
> >> [    8.114202]  bpf_check+0x3bc/0x28c0
> >> [    8.114214]  bpf_prog_load+0x658/0xa58
> >> [    8.114227]  __sys_bpf+0xc50/0x2250
> >> [    8.114240]  __arm64_sys_bpf+0x28/0x40
> >> [    8.114254]  invoke_syscall.constprop.0+0x54/0xf0
> >> [    8.114273]  do_el0_svc+0x4c/0xd8
> >> [    8.114289]  el0_svc+0x3c/0x140
> >> [    8.114305]  el0t_64_sync_handler+0x134/0x150
> >> [    8.114331]  el0t_64_sync+0x168/0x170
> >> [    8.114477] Code: 7100707f 54000081 f9401c00 f9403800 (b9400403)
> >> [    8.118672] ---[ end trace 0000000000000000 ]---
> >>
> >> Fix this by adding dst_prog non-empty check in BPF_PROG_TYPE_EXT case
> >> when calling bpf_prog_load().
> >>
> >> Fixes: 4a9c7bbe2ed4 ("bpf: Resolve to prog->aux->dst_prog->type only for BPF_PROG_TYPE_EXT")
> >> Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
> >> Cc: stable@vger.kernel.org # v5.18+
> >> ---
> >>  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 f45ed6adc092..4490f8ccf006 100644
> >> --- a/kernel/bpf/syscall.c
> >> +++ b/kernel/bpf/syscall.c
> >> @@ -2632,9 +2632,12 @@ bpf_prog_load_check_attach(enum bpf_prog_type prog_type,
> >>  			return 0;
> >>  		return -EINVAL;
> >>  	case BPF_PROG_TYPE_SYSCALL:
> >> -	case BPF_PROG_TYPE_EXT:
> >>  		if (expected_attach_type)
> >>  			return -EINVAL;
> >> +		return 0;
> >> +	case BPF_PROG_TYPE_EXT:
> >> +		if (expected_attach_type || !dst_prog)
> >> +			return -EINVAL;
> >>  		fallthrough;
> >>  	default:
> >>  		return 0;
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf] bpf: Fix null pointer dereference in resolve_prog_type() for BPF_PROG_TYPE_EXT
  2024-06-20  6:07 [PATCH bpf] bpf: Fix null pointer dereference in resolve_prog_type() for BPF_PROG_TYPE_EXT Tengda Wu
  2024-06-20  6:46 ` Leon Hwang
@ 2024-06-20 10:28 ` Leon Hwang
  1 sibling, 0 replies; 5+ messages in thread
From: Leon Hwang @ 2024-06-20 10:28 UTC (permalink / raw)
  To: Tengda Wu, bpf
  Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa



On 20/6/24 14:07, Tengda Wu wrote:
> When loading a EXT program without specifying `attr->attach_prog_fd`,
> the `prog->aux->dst_prog` will be null. At this time, calling
> resolve_prog_type() anywhere will result in a null pointer dereference.
> 
> Example stack trace:
> 
> [    8.107863] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000004
> [    8.108262] Mem abort info:
> [    8.108384]   ESR = 0x0000000096000004
> [    8.108547]   EC = 0x25: DABT (current EL), IL = 32 bits
> [    8.108722]   SET = 0, FnV = 0
> [    8.108827]   EA = 0, S1PTW = 0
> [    8.108939]   FSC = 0x04: level 0 translation fault
> [    8.109102] Data abort info:
> [    8.109203]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> [    8.109399]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> [    8.109614]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [    8.109836] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000101354000
> [    8.110011] [0000000000000004] pgd=0000000000000000, p4d=0000000000000000
> [    8.112624] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
> [    8.112783] Modules linked in:
> [    8.113120] CPU: 0 PID: 99 Comm: may_access_dire Not tainted 6.10.0-rc3-next-20240613-dirty #1
> [    8.113230] Hardware name: linux,dummy-virt (DT)
> [    8.113390] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [    8.113429] pc : may_access_direct_pkt_data+0x24/0xa0
> [    8.113746] lr : add_subprog_and_kfunc+0x634/0x8e8
> [    8.113798] sp : ffff80008283b9f0
> [    8.113813] x29: ffff80008283b9f0 x28: ffff800082795048 x27: 0000000000000001
> [    8.113881] x26: ffff0000c0bb2600 x25: 0000000000000000 x24: 0000000000000000
> [    8.113897] x23: ffff0000c1134000 x22: 000000000001864f x21: ffff0000c1138000
> [    8.113912] x20: 0000000000000001 x19: ffff0000c12b8000 x18: ffffffffffffffff
> [    8.113929] x17: 0000000000000000 x16: 0000000000000000 x15: 0720072007200720
> [    8.113944] x14: 0720072007200720 x13: 0720072007200720 x12: 0720072007200720
> [    8.113958] x11: 0720072007200720 x10: 0000000000f9fca4 x9 : ffff80008021f4e4
> [    8.113991] x8 : 0101010101010101 x7 : 746f72705f6d656d x6 : 000000001e0e0f5f
> [    8.114006] x5 : 000000000001864f x4 : ffff0000c12b8000 x3 : 000000000000001c
> [    8.114020] x2 : 0000000000000002 x1 : 0000000000000000 x0 : 0000000000000000
> [    8.114126] Call trace:
> [    8.114159]  may_access_direct_pkt_data+0x24/0xa0
> [    8.114202]  bpf_check+0x3bc/0x28c0
> [    8.114214]  bpf_prog_load+0x658/0xa58
> [    8.114227]  __sys_bpf+0xc50/0x2250
> [    8.114240]  __arm64_sys_bpf+0x28/0x40
> [    8.114254]  invoke_syscall.constprop.0+0x54/0xf0
> [    8.114273]  do_el0_svc+0x4c/0xd8
> [    8.114289]  el0_svc+0x3c/0x140
> [    8.114305]  el0t_64_sync_handler+0x134/0x150
> [    8.114331]  el0t_64_sync+0x168/0x170
> [    8.114477] Code: 7100707f 54000081 f9401c00 f9403800 (b9400403)
> [    8.118672] ---[ end trace 0000000000000000 ]---
> 
> Fix this by adding dst_prog non-empty check in BPF_PROG_TYPE_EXT case
> when calling bpf_prog_load().
> 
> Fixes: 4a9c7bbe2ed4 ("bpf: Resolve to prog->aux->dst_prog->type only for BPF_PROG_TYPE_EXT")
> Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
> Cc: stable@vger.kernel.org # v5.18+
> ---
>  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 f45ed6adc092..4490f8ccf006 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2632,9 +2632,12 @@ bpf_prog_load_check_attach(enum bpf_prog_type prog_type,
>  			return 0;
>  		return -EINVAL;
>  	case BPF_PROG_TYPE_SYSCALL:
> -	case BPF_PROG_TYPE_EXT:
>  		if (expected_attach_type)
>  			return -EINVAL;
> +		return 0;
> +	case BPF_PROG_TYPE_EXT:
> +		if (expected_attach_type || !dst_prog)
> +			return -EINVAL;
>  		fallthrough;
>  	default:
>  		return 0;

Would be better to add a selftest for it.
But, looks good to me.

Acked-by: Leon Hwang <hffilwlqm@gmail.com>


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-06-20 10:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-20  6:07 [PATCH bpf] bpf: Fix null pointer dereference in resolve_prog_type() for BPF_PROG_TYPE_EXT Tengda Wu
2024-06-20  6:46 ` Leon Hwang
2024-06-20  8:54   ` Tengda Wu
2024-06-20 10:27     ` Jiri Olsa
2024-06-20 10:28 ` Leon Hwang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox