bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Krister Johansen <kjlx@templeofstupid.com>
To: bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	John Fastabend <john.fastabend@gmail.com>,
	Andrii Nakryiko <andrii@kernel.org>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>,
	KP Singh <kpsingh@kernel.org>,
	Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>,
	Jiri Olsa <jolsa@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Jesper Dangaard Brouer <hawk@kernel.org>,
	Nathan Chancellor <nathan@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Tom Rix <trix@redhat.com>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	llvm@lists.linux.dev, stable@vger.kernel.org
Subject: [PATCH bpf] bpf: search_bpf_extables should search subprogram extables
Date: Mon, 5 Jun 2023 09:49:55 -0700	[thread overview]
Message-ID: <20230605164955.GA1977@templeofstupid.com> (raw)

JIT'd bpf programs that have subprograms can have a postive value for
num_extentries but a NULL value for extable.  This is problematic if one of
these bpf programs encounters a fault during its execution.  The fault
handlers correctly identify that the faulting IP belongs to a bpf program.
However, performing a search_extable call on a NULL extable leads to a
second fault.

Fix up by refusing to search a NULL extable, and by checking the
subprograms' extables if the umbrella program has subprograms configured.

Once I realized what was going on, I was able to use the following bpf
program to get an oops from this failure:

   #include "vmlinux.h"
   #include <bpf/bpf_helpers.h>
   #include <bpf/bpf_tracing.h>
   
   char LICENSE[] SEC("license") = "Dual BSD/GPL";
   
   #define PATH_MAX 4096
   
   struct callback_ctx {
           u8 match;
   };
   
   struct filter_value {
           char prefix[PATH_MAX];
   };
   struct {
           __uint(type, BPF_MAP_TYPE_ARRAY);
           __uint(max_entries, 256);
           __type(key, int);
           __type(value, struct filter_value);
   } test_filter SEC(".maps");
   
   static __u64 test_filter_cb(struct bpf_map *map, __u32 *key,
                               struct filter_value *val,
                               struct callback_ctx *data)
   {
       return 1;
   }
   
   SEC("fentry/__sys_bind")
   int BPF_PROG(__sys_bind, int fd, struct sockaddr *umyaddr, int addrlen)
   {
     pid_t pid;
   
     struct callback_ctx cx = { .match = 0 };
     pid = bpf_get_current_pid_tgid() >> 32;
     bpf_for_each_map_elem(&test_filter, test_filter_cb, &cx, 0);
     bpf_printk("fentry: pid = %d, family = %llx\n", pid, umyaddr->sa_family);
     return 0;
   }

And then the following code to actually trigger a failure:

  #include <stdio.h>
  #include <stdlib.h>
  #include <unistd.h>
  #include <sys/socket.h>
  #include <netinet/in.h>
  #include <netinet/ip.h>
  
  int
  main(int argc, char *argv[])
  {
    int sfd, rc;
    struct sockaddr *sockptr = (struct sockaddr *)0x900000000000;
  
    sfd = socket(AF_INET, SOCK_STREAM, 0);
    if (sfd < 0) {
      perror("socket");
      exit(EXIT_FAILURE);
    }
  
    while (1) {
      rc = bind(sfd, (struct sockaddr *) sockptr, sizeof(struct sockaddr_in));
      if (rc < 0) {
        perror("bind");
        sleep(5);
      } else {
        break;
      }
    }
  
    return 0;
  }

I was able to validate that this problem does not occur when subprograms
are not in use, or when the direct pointer accesses are replaced with
bpf_probe_read calls.  I further validated that this did not break the
extable handling in existing bpf programs.  The same program caused no
failures when subprograms were removed, but the exception was still
injected.

Cc: stable@vger.kernel.org
Fixes: 1c2a088a6626 ("bpf: x64: add JIT support for multi-function programs")
Signed-off-by: Krister Johansen <kjlx@templeofstupid.com>
---
 kernel/bpf/core.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 7421487422d4..0e12238e4340 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -736,15 +736,33 @@ const struct exception_table_entry *search_bpf_extables(unsigned long addr)
 {
 	const struct exception_table_entry *e = NULL;
 	struct bpf_prog *prog;
+	struct bpf_prog_aux *aux;
+	int i;
 
 	rcu_read_lock();
 	prog = bpf_prog_ksym_find(addr);
 	if (!prog)
 		goto out;
-	if (!prog->aux->num_exentries)
+	aux = prog->aux;
+	if (!aux->num_exentries)
 		goto out;
 
-	e = search_extable(prog->aux->extable, prog->aux->num_exentries, addr);
+	/* prog->aux->extable can be NULL if subprograms are in use. In that
+	 * case, check each sub-function's aux->extables to see if it has a
+	 * matching entry.
+	 */
+	if (aux->extable != NULL) {
+		e = search_extable(prog->aux->extable,
+		    prog->aux->num_exentries, addr);
+	} else {
+		for (i = 0; (i < aux->func_cnt) && (e == NULL); i++) {
+			if (!aux->func[i]->aux->num_exentries ||
+			    aux->func[i]->aux->extable == NULL)
+				continue;
+			e = search_extable(aux->func[i]->aux->extable,
+			    aux->func[i]->aux->num_exentries, addr);
+		}
+	}
 out:
 	rcu_read_unlock();
 	return e;
-- 
2.25.1


             reply	other threads:[~2023-06-05 16:50 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-05 16:49 Krister Johansen [this message]
2023-06-05 23:30 ` [PATCH bpf] bpf: search_bpf_extables should search subprogram extables Alexei Starovoitov
2023-06-06  0:41   ` Krister Johansen
2023-06-06  1:31     ` Alexei Starovoitov
2023-06-07 21:04       ` Krister Johansen

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=20230605164955.GA1977@templeofstupid.com \
    --to=kjlx@templeofstupid.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=haoluo@google.com \
    --cc=hawk@kernel.org \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=martin.lau@linux.dev \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=sdf@google.com \
    --cc=song@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=trix@redhat.com \
    --cc=yhs@fb.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;
as well as URLs for NNTP newsgroup(s).