All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eduard Zingerman <eddyz87@gmail.com>
To: bpf@vger.kernel.org, ast@kernel.org
Cc: andrii@kernel.org, daniel@iogearbox.net, martin.lau@linux.dev,
	kernel-team@fb.com, yonghong.song@linux.dev, mejedi@gmail.com,
	Eduard Zingerman <eddyz87@gmail.com>
Subject: [PATCH bpf 0/4] bpf: track changes_pkt_data property for global functions
Date: Thu,  5 Dec 2024 20:03:03 -0800	[thread overview]
Message-ID: <20241206040307.568065-1-eddyz87@gmail.com> (raw)

Nick Zavaritsky reported [0] a bug in verifier, where the following
unsafe program is not rejected:

    __attribute__((__noinline__))
    long skb_pull_data(struct __sk_buff *sk, __u32 len)
    {
        return bpf_skb_pull_data(sk, len);
    }

    SEC("tc")
    int test_invalidate_checks(struct __sk_buff *sk)
    {
        int *p = (void *)(long)sk->data;
        if ((void *)(p + 1) > (void *)(long)sk->data_end) return TCX_DROP;
        skb_pull_data(sk, 0);
        /* not safe, p is invalid after bpf_skb_pull_data call */
        *p = 42;
        return TCX_PASS;
    }

This happens because verifier does not track package invalidation
effect of global sub-programs.

This patch-set fixes the issue by modifying check_cfg() to compute
whether or not each sub-program calls (directly or indirectly)
helper invalidating packet pointers.

[0] https://lore.kernel.org/bpf/0498CA22-5779-4767-9C0C-A9515CEA711F@gmail.com/

Eduard Zingerman (4):
  bpf: add find_containing_subprog() utility function
  bpf: refactor bpf_helper_changes_pkt_data to use helper number
  bpf: track changes_pkt_data property for global functions
  selftests/bpf: test for changing packet data from global functions

 include/linux/bpf_verifier.h                  |  1 +
 include/linux/filter.h                        |  2 +-
 kernel/bpf/core.c                             |  2 +-
 kernel/bpf/verifier.c                         | 62 ++++++++++++++++--
 net/core/filter.c                             | 63 +++++++++----------
 .../selftests/bpf/progs/verifier_sock.c       | 28 +++++++++
 6 files changed, 115 insertions(+), 43 deletions(-)

-- 
2.47.0


             reply	other threads:[~2024-12-06  4:03 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-06  4:03 Eduard Zingerman [this message]
2024-12-06  4:03 ` [PATCH bpf 1/4] bpf: add find_containing_subprog() utility function Eduard Zingerman
2024-12-06  4:03 ` [PATCH bpf 2/4] bpf: refactor bpf_helper_changes_pkt_data to use helper number Eduard Zingerman
2024-12-06  4:03 ` [PATCH bpf 3/4] bpf: track changes_pkt_data property for global functions Eduard Zingerman
2024-12-06 20:43   ` Alexei Starovoitov
2024-12-06 21:35     ` Eduard Zingerman
2024-12-09  7:40     ` Eduard Zingerman
2024-12-09 16:53       ` Alexei Starovoitov
2024-12-09 17:57         ` Eduard Zingerman
2024-12-10  0:48           ` Alexei Starovoitov
2024-12-10  1:24             ` Eduard Zingerman
2024-12-10  1:38               ` Alexei Starovoitov
2024-12-06  4:03 ` [PATCH bpf 4/4] selftests/bpf: test for changing packet data from " Eduard Zingerman

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=20241206040307.568065-1-eddyz87@gmail.com \
    --to=eddyz87@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@fb.com \
    --cc=martin.lau@linux.dev \
    --cc=mejedi@gmail.com \
    --cc=yonghong.song@linux.dev \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.