BPF List
 help / color / mirror / Atom feed
From: "Emil Tsalapatis" <emil@etsalapatis.com>
To: "Amery Hung" <ameryhung@gmail.com>,
	"Emil Tsalapatis" <emil@etsalapatis.com>
Cc: <bpf@vger.kernel.org>, <ast@kernel.org>, <andrii@kernel.org>,
	<eddyz87@gmail.com>, <memxor@gmail.com>, <daniel@iogearbox.net>,
	<netdev@vger.kernel.org>,
	"Nicholas Carlini" <nicholas@carlini.com>,
	"Mahe Tardy" <mahe.tardy@gmail.com>
Subject: Re: [PATCH bpf 05/11] bpf: Reject pkt arguments in mutating subprogs
Date: Wed, 16 Sep 2026 18:41:03 +0000	[thread overview]
Message-ID: <DLGYLT5UIFDA.38RG9UBZNYI58@etsalapatis.com> (raw)
In-Reply-To: <CAMB2axOAut_9utesiV9mG40bskrDscu+Gu9FUMNRKZC+hbd8UQ@mail.gmail.com>

On Wed Sep 16, 2026 at 5:57 AM UTC, Amery Hung wrote:
> On Tue, Sep 15, 2026 at 10:10 PM Emil Tsalapatis <emil@etsalapatis.com> wrote:
>>
>> The verifier tracks changes in how PTR_TO_PACKET registers'
>> bounds are modified across subprog boundaries. PTR_TO_PACKET
>> registers are actually passed as PTR_TO_MEM, which is assumed
>> valid for the entire call. This is not the case with packet memory,
>> where a pskb_* call may invalidate its memory region.
>>
>> Reject BPF code that passes PTR_TO_PACKET pointers to subprogs that
>> may mutate a packet. We cannot pass the pointer as a true PTR_TO_PACKET
>> because we would also need to somehow pass the PTR_TO_PACKET_META
>> or PTR_TO_PACKET_END to the subprog. Since we cannot avoid representing
>> the pointer in the subprog as PTR_TO_MEM, only permit it if the
>> subprog is guaranteed not to mutate the packet.
>
> CC Mahe.
>
> Hi Emil,
>
> There is a related but separate issue [1] addressed by 8fe994c80af2
> (“bpf: Consolidate function call pkt_access validation”). I think a
> long-term solution could be supporting ARG_PTR_TO_PACKET for global
> subprograms. For example:
>
> int parse_something(struct __sk_buff *skb, __u16 off,
>                     char *data_start __arg_packet, ...);
>
> The verifier could require a real PTR_TO_PACKET at the call site and
> verify the global subprogram with a real PTR_TO_PACKET, rather than
> converting it to PTR_TO_MEM. This would preserve packet access rules,
> allow reads from cgroup_skb programs while rejecting writes in the
> callee, and automatically invalidate the argument after calls such as
> bpf_skb_pull_data().
>

Hi Amery,

    that makes sense to me, especially since this is a feature expected
by existing programs. The main issue I see is that since we want to be able
to pass it with subprogs we need to find a way to annotate its current size
(maybe passing it as a __sz that is checked by the verifier to be <= the
range - off of the pointer in the caller?)

> [1] https://lore.kernel.org/bpf/aqkhifLvyAhw66jg@gmail.com/#t
>
>>
>> Fixes: 80f281664f5a ("bpf: Support pointers in global func args")
>> Reported-by: Nicholas Carlini <nicholas@carlini.com>
>> Suggested-by: Nicholas Carlini <nicholas@carlini.com>
>> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
>> ---
>>  kernel/bpf/verifier.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 6c6b8d852..507bc14b4 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -10375,6 +10375,16 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
>>                         if (check_mem_reg(env, reg, argno, arg->mem_size, BPF_READ | BPF_WRITE, NULL,
>>                                           NULL))
>>                                 return -EINVAL;
>> +                       /*
>> +                        * PTR_TO_PACKET get passed as PTR_TO_MEM, preventing
>> +                        * us from adjusting bounds tracking info.
>> +                        */
>> +                       if ((reg_is_pkt_pointer_any(reg) || reg_is_dynptr_slice_pkt(reg)) &&
>> +                           sub->changes_pkt_data) {
>> +                               bpf_log(log, "%s is a packet pointer, but func#%d may change packet data\n",
>> +                                               reg_arg_name(env, argno), subprog);
>> +                               return -EINVAL;
>> +                       }
>>                         if (!(arg->arg_type & PTR_MAYBE_NULL) &&
>>                             (type_may_be_null(reg->type) || bpf_register_is_null(reg))) {
>>                                 bpf_log(log, "%s is expected to be non-NULL\n",
>> --
>> 2.54.0
>>
>>


  parent reply	other threads:[~2026-09-16 18:41 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-16  5:08 [PATCH bpf 00/11] skb/arena bugfixes Emil Tsalapatis
2026-09-16  5:08 ` [PATCH bpf 01/11] bpf: Fix bounds check for skb-backed dynptrs Emil Tsalapatis
2026-09-16 11:52   ` Jiayuan Chen
2026-09-16 23:08   ` Jakub Kicinski
2026-09-16  5:08 ` [PATCH bpf 02/11] selftests/bpf: Test dynptr slices past end of skb Emil Tsalapatis
2026-09-16  5:18   ` sashiko-bot
2026-09-16  5:08 ` [PATCH bpf 03/11] bpf: Fix bpf_sock context code generation Emil Tsalapatis
2026-09-16 12:21   ` Jiayuan Chen
2026-09-16  5:08 ` [PATCH bpf 04/11] selftests/bpf: Add selftests for rx_queue_mapping context access Emil Tsalapatis
2026-09-16  5:08 ` [PATCH bpf 05/11] bpf: Reject pkt arguments in mutating subprogs Emil Tsalapatis
2026-09-16  5:57   ` Amery Hung
2026-09-16  5:58     ` Amery Hung
2026-09-16 18:41     ` Emil Tsalapatis [this message]
2026-09-16 19:26       ` Amery Hung
2026-09-16  5:08 ` [PATCH bpf 06/11] selftests/bpf: Test rejection of pkt args to " Emil Tsalapatis
2026-09-16  5:08 ` [PATCH bpf 07/11] bpf: Prevent variable arena/non-arena register contents Emil Tsalapatis
2026-09-16  5:08 ` [PATCH bpf 08/11] selftests/bpf: Test for mixed arena/nonarena code paths Emil Tsalapatis
2026-09-16  5:17   ` sashiko-bot
2026-09-16  5:08 ` [PATCH bpf 09/11] bpf: Track whether dynptr type is known Emil Tsalapatis
2026-09-16  5:22   ` sashiko-bot
2026-09-16 18:41     ` Emil Tsalapatis
2026-09-16  5:08 ` [PATCH bpf 10/11] bpf: Track skb memory invalidation by packet-backed dynptrs Emil Tsalapatis
2026-09-16  5:08 ` [PATCH bpf 11/11] selftests/bpf: Test dynptr slice invalidation on skb clobber Emil Tsalapatis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DLGYLT5UIFDA.38RG9UBZNYI58@etsalapatis.com \
    --to=emil@etsalapatis.com \
    --cc=ameryhung@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=mahe.tardy@gmail.com \
    --cc=memxor@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=nicholas@carlini.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox