linux-kernel - Re: [RFC PATCH bpf-next RESEND 03/16] bpf: Improve bpf kfuncs pointer arguments chain of trust

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAP01T75ZUE04VKFrjL=uW-j+9DLF6OnHuzGBWjfjf4zGnuq7aQ@mail.gmail.com>
Date: Wed, 31 Jul 2024 18:35:33 +0200
From: Kumar Kartikeya Dwivedi <memxor@...il.com>
To: Juntong Deng <juntong.deng@...look.com>
Cc: ast@...nel.org, daniel@...earbox.net, john.fastabend@...il.com, 
	martin.lau@...ux.dev, eddyz87@...il.com, song@...nel.org, 
	yonghong.song@...ux.dev, kpsingh@...nel.org, sdf@...ichev.me, 
	haoluo@...gle.com, jolsa@...nel.org, andrii@...nel.org, avagin@...il.com, 
	snorcht@...il.com, bpf@...r.kernel.org, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH bpf-next RESEND 03/16] bpf: Improve bpf kfuncs pointer
 arguments chain of trust

On Wed, 31 Jul 2024 at 15:29, Juntong Deng <juntong.deng@...look.com> wrote:
>
> On 7/23/24 01:20, Kumar Kartikeya Dwivedi wrote:
> > On Thu, 11 Jul 2024 at 13:26, Juntong Deng <juntong.deng@...look.com> wrote:
> >>
> >> [...]
> >>
> >> 2. sk_write_queue in struct sock
> >> sk_write_queue is a struct member in struct sock, not a pointer
> >> member, so we cannot use the guaranteed valid nested pointer method
> >> to get a valid pointer to sk_write_queue.
> >
> > I think Matt recently had a patch addressing this issue:
> > https://lore.kernel.org/bpf/20240709210939.1544011-1-mattbobrowski@google.com/
> > I believe that should resolve this one (as far as passing them into
> > KF_TRUSTED_ARGS kfuncs is concerned atleast).
> >
>
> Thanks for letting me know.
>
> I tested it and it works well in most cases, but there are a few cases
> that are not fully resolved.
>
> Yes, the verifier has relaxed the constraint on non-zero offset
> pointers, but the type of the pointer does not change.
>
> This means that passing non-zero offset pointers as arguments to kfuncs
> with KF_ACQUIRE will be rejected by the verifier because KF_ACQUIRE
> requires strict type match and casting cannot be used.
>
> An example, bpf_skb_peek_tail:
>
> # ; struct sk_buff *skb = bpf_skb_peek_tail(head);
> @ test_restore_udp_socket.bpf.c:209
>
> # 75: (bf) r1 = r2                      ;
> frame2: R1_w=ptr_sock(ref_obj_id=6,off=168)
> R2=ptr_sock(ref_obj_id=6,off=168) refs=4,6
>
> # 76: (85) call bpf_skb_peek_tail#101113
> # kernel function bpf_skb_peek_tail args#0 expected pointer to
> STRUCT sk_buff_head but R1 has a pointer to STRUCT sock
>
> Should we relax the strict type-matching constraint on non-zero offset
> pointers when used as arguments to kfuncs with KF_ACQUIRE?
>

Yes, I think it makes sense (on surface, though it requires some
deliberation, can do it over the exact code).
As long as we prevent special cases through existing
btf_type_ids_nocast_alias, it should be ok.
Please send a separate patch for this, and include selftests
exercising corner cases.

>
> In addition, this method is not portable (such as &task->cpus_mask),
> and the offset of the member will change once a new structure member
> is added, or an old structure member is removed.

This should be handled by BPF CO-RE. A lot of selftests would fail if
this didn't work.
As long as the struct is annotated as __attribute__((preserve_access_index)).

>
> [...]