bpf: Enforce NULL check on new _OR_NULL return types

.travis.yml

@@ -0,0 +1,33 @@
+sudo: required
+language: bash
+dist: bionic
+services:
+    - docker
+
+env:
+    global:
+        - PROJECT_NAME='libbpf'
+        - AUTHOR_EMAIL="$(git log -1 --pretty=\"%aE\")"
+        - REPO_ROOT="$TRAVIS_BUILD_DIR"
+        - CI_ROOT="$REPO_ROOT/travis-ci"
+        - VMTEST_ROOT="$CI_ROOT/vmtest"
+
+addons:
+    apt:
+        packages:
+            - qemu-kvm
+            - zstd
+            - binutils-dev
+            - elfutils
+            - libcap-dev
+            - libelf-dev
+            - libdw-dev
+            - python3-docutils
+
+jobs:
+    include:
+        - stage: Builds & Tests
+          name: Kernel LATEST + selftests
+          language: bash
+          env: KERNEL=LATEST
+          script:  $CI_ROOT/vmtest/run_vmtest.sh || travis_terminate 1

kernel/bpf/verifier.c

@@ -5133,24 +5133,19 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
 				regs[BPF_REG_0].id = ++env->id_gen;
 		} else {
 			regs[BPF_REG_0].type = PTR_TO_MAP_VALUE_OR_NULL;
-			regs[BPF_REG_0].id = ++env->id_gen;
 		}
 	} else if (fn->ret_type == RET_PTR_TO_SOCKET_OR_NULL) {
 		mark_reg_known_zero(env, regs, BPF_REG_0);
 		regs[BPF_REG_0].type = PTR_TO_SOCKET_OR_NULL;
-		regs[BPF_REG_0].id = ++env->id_gen;
 	} else if (fn->ret_type == RET_PTR_TO_SOCK_COMMON_OR_NULL) {
 		mark_reg_known_zero(env, regs, BPF_REG_0);
 		regs[BPF_REG_0].type = PTR_TO_SOCK_COMMON_OR_NULL;
-		regs[BPF_REG_0].id = ++env->id_gen;
 	} else if (fn->ret_type == RET_PTR_TO_TCP_SOCK_OR_NULL) {
 		mark_reg_known_zero(env, regs, BPF_REG_0);
 		regs[BPF_REG_0].type = PTR_TO_TCP_SOCK_OR_NULL;
-		regs[BPF_REG_0].id = ++env->id_gen;
 	} else if (fn->ret_type == RET_PTR_TO_ALLOC_MEM_OR_NULL) {
 		mark_reg_known_zero(env, regs, BPF_REG_0);
 		regs[BPF_REG_0].type = PTR_TO_MEM_OR_NULL;
-		regs[BPF_REG_0].id = ++env->id_gen;
 		regs[BPF_REG_0].mem_size = meta.mem_size;
 	} else if (fn->ret_type == RET_PTR_TO_MEM_OR_BTF_ID_OR_NULL ||
 		   fn->ret_type == RET_PTR_TO_MEM_OR_BTF_ID) {
@@ -5199,6 +5194,9 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
 		return -EINVAL;
 	}
 
+	if (reg_type_may_be_null(regs[BPF_REG_0].type))
+		regs[BPF_REG_0].id = ++env->id_gen;
+
 	if (is_ptr_cast_function(func_id)) {
 		/* For release_reference() */
 		regs[BPF_REG_0].ref_obj_id = meta.ref_obj_id;
@@ -7212,7 +7210,8 @@ static void mark_ptr_or_null_reg(struct bpf_func_state *state,
 				 struct bpf_reg_state *reg, u32 id,
 				 bool is_null)
 {
-	if (reg_type_may_be_null(reg->type) && reg->id == id) {
+	if (reg_type_may_be_null(reg->type) && reg->id == id &&
+	    !WARN_ON_ONCE(!reg->id)) {
 		/* Old offset (both fixed and variable parts) should
 		 * have been known-zero, because we don't allow pointer
 		 * arithmetic on pointers that might be NULL.

tools/testing/selftests/bpf/prog_tests/ksyms_btf.c

@@ -5,18 +5,17 @@
 #include <bpf/libbpf.h>
 #include <bpf/btf.h>
 #include "test_ksyms_btf.skel.h"
+#include "test_ksyms_btf_null_check.skel.h"
 
 static int duration;
 
-void test_ksyms_btf(void)
+static void test_basic(void)
 {
 	__u64 runqueues_addr, bpf_prog_active_addr;
 	__u32 this_rq_cpu;
 	int this_bpf_prog_active;
 	struct test_ksyms_btf *skel = NULL;
 	struct test_ksyms_btf__data *data;
-	struct btf *btf;
-	int percpu_datasec;
 	int err;
 
 	err = kallsyms_find("runqueues", &runqueues_addr);
@@ -31,20 +30,6 @@ void test_ksyms_btf(void)
 	if (CHECK(err == -ENOENT, "ksym_find", "symbol 'bpf_prog_active' not found\n"))
 		return;
 
-	btf = libbpf_find_kernel_btf();
-	if (CHECK(IS_ERR(btf), "btf_exists", "failed to load kernel BTF: %ld\n",
-		  PTR_ERR(btf)))
-		return;
-
-	percpu_datasec = btf__find_by_name_kind(btf, ".data..percpu",
-						BTF_KIND_DATASEC);
-	if (percpu_datasec < 0) {
-		printf("%s:SKIP:no PERCPU DATASEC in kernel btf\n",
-		       __func__);
-		test__skip();
-		goto cleanup;
-	}
-
 	skel = test_ksyms_btf__open_and_load();
 	if (CHECK(!skel, "skel_open", "failed to open and load skeleton\n"))
 		goto cleanup;
@@ -83,6 +68,42 @@ void test_ksyms_btf(void)
 	      data->out__bpf_prog_active);
 
 cleanup:
-	btf__free(btf);
 	test_ksyms_btf__destroy(skel);
 }
+
+static void test_null_check(void)
+{
+	struct test_ksyms_btf_null_check *skel;
+
+	skel = test_ksyms_btf_null_check__open_and_load();
+	CHECK(skel, "skel_open", "unexpected load of a prog missing null check\n");
+
+	test_ksyms_btf_null_check__destroy(skel);
+}
+
+void test_ksyms_btf(void)
+{
+	int percpu_datasec;
+	struct btf *btf;
+
+	btf = libbpf_find_kernel_btf();
+	if (CHECK(IS_ERR(btf), "btf_exists", "failed to load kernel BTF: %ld\n",
+		  PTR_ERR(btf)))
+		return;
+
+	percpu_datasec = btf__find_by_name_kind(btf, ".data..percpu",
+						BTF_KIND_DATASEC);
+	btf__free(btf);
+	if (percpu_datasec < 0) {
+		printf("%s:SKIP:no PERCPU DATASEC in kernel btf\n",
+		       __func__);
+		test__skip();
+		return;
+	}
+
+	if (test__start_subtest("basic"))
+		test_basic();
+
+	if (test__start_subtest("null_check"))
+		test_null_check();
+}

tools/testing/selftests/bpf/progs/test_ksyms_btf_null_check.c

@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2020 Facebook */
+
+#include "vmlinux.h"
+
+#include <bpf/bpf_helpers.h>
+
+extern const struct rq runqueues __ksym; /* struct type global var. */
+extern const int bpf_prog_active __ksym; /* int type global var. */
+
+SEC("raw_tp/sys_enter")
+int handler(const void *ctx)
+{
+	struct rq *rq;
+	int *active;
+	__u32 cpu;
+
+	cpu = bpf_get_smp_processor_id();
+	rq = (struct rq *)bpf_per_cpu_ptr(&runqueues, cpu);
+	active = (int *)bpf_per_cpu_ptr(&bpf_prog_active, cpu);
+	if (active) {
+		/* READ_ONCE */
+		*(volatile int *)active;
+		/* !rq has not been tested, so verifier should reject. */
+		*(volatile int *)(&rq->cpu);
+	}
+
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";

tools/testing/selftests/bpf/verifier/sock.c

@@ -631,3 +631,28 @@
 	.prog_type = BPF_PROG_TYPE_SK_REUSEPORT,
 	.result = ACCEPT,
 },
+{
+	"mark null check on return value of bpf_skc_to helpers",
+	.insns = {
+	BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, offsetof(struct __sk_buff, sk)),
+	BPF_JMP_IMM(BPF_JNE, BPF_REG_1, 0, 2),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
+	BPF_EMIT_CALL(BPF_FUNC_skc_to_tcp_sock),
+	BPF_MOV64_REG(BPF_REG_7, BPF_REG_0),
+	BPF_MOV64_REG(BPF_REG_1, BPF_REG_6),
+	BPF_EMIT_CALL(BPF_FUNC_skc_to_tcp_request_sock),
+	BPF_MOV64_REG(BPF_REG_8, BPF_REG_0),
+	BPF_JMP_IMM(BPF_JNE, BPF_REG_8, 0, 2),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_7, 0),
+	BPF_EXIT_INSN(),
+	},
+	.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+	.result = REJECT,
+	.errstr = "invalid mem access",
+	.result_unpriv = REJECT,
+	.errstr_unpriv = "unknown func",
+},

travis-ci/managers/debian.sh

@@ -0,0 +1,84 @@
+#!/bin/bash
+
+PHASES=(${@:-SETUP RUN RUN_ASAN CLEANUP})
+DEBIAN_RELEASE="${DEBIAN_RELEASE:-testing}"
+CONT_NAME="${CONT_NAME:-libbpf-debian-$DEBIAN_RELEASE}"
+ENV_VARS="${ENV_VARS:-}"
+DOCKER_RUN="${DOCKER_RUN:-docker run}"
+REPO_ROOT="${REPO_ROOT:-$PWD}"
+ADDITIONAL_DEPS=(clang pkg-config gcc-8)
+CFLAGS="-g -O2 -Werror -Wall"
+
+function info() {
+    echo -e "\033[33;1m$1\033[0m"
+}
+
+function error() {
+    echo -e "\033[31;1m$1\033[0m"
+}
+
+function docker_exec() {
+    docker exec $ENV_VARS -it $CONT_NAME "$@"
+}
+
+set -e
+
+source "$(dirname $0)/travis_wait.bash"
+
+for phase in "${PHASES[@]}"; do
+    case $phase in
+        SETUP)
+            info "Setup phase"
+            info "Using Debian $DEBIAN_RELEASE"
+
+            sudo apt-get -y -o Dpkg::Options::="--force-confnew" install docker-ce
+            docker --version
+
+            docker pull debian:$DEBIAN_RELEASE
+            info "Starting container $CONT_NAME"
+            $DOCKER_RUN -v $REPO_ROOT:/build:rw \
+                        -w /build --privileged=true --name $CONT_NAME \
+                        -dit --net=host debian:$DEBIAN_RELEASE /bin/bash
+            docker_exec bash -c "echo deb-src http://deb.debian.org/debian $DEBIAN_RELEASE main >>/etc/apt/sources.list"
+            docker_exec apt-get -y update
+            docker_exec apt-get -y build-dep libelf-dev
+            docker_exec apt-get -y install libelf-dev
+            docker_exec apt-get -y install "${ADDITIONAL_DEPS[@]}"
+            ;;
+        RUN|RUN_CLANG|RUN_GCC8|RUN_ASAN|RUN_CLANG_ASAN|RUN_GCC8_ASAN)
+            if [[ "$phase" = *"CLANG"* ]]; then
+                ENV_VARS="-e CC=clang -e CXX=clang++"
+                CC="clang"
+            elif [[ "$phase" = *"GCC8"* ]]; then
+                ENV_VARS="-e CC=gcc-8 -e CXX=g++-8"
+                CC="gcc-8"
+            else
+                CFLAGS="${CFLAGS} -Wno-stringop-truncation"
+            fi
+            if [[ "$phase" = *"ASAN"* ]]; then
+                CFLAGS="${CFLAGS} -fsanitize=address,undefined"
+            fi
+            docker_exec mkdir build install
+            docker_exec ${CC:-cc} --version
+            info "build"
+	    docker_exec make -j$((4*$(nproc))) CFLAGS="${CFLAGS}" -C ./src -B OBJDIR=../build
+            info "ldd build/libbpf.so:"
+            docker_exec ldd build/libbpf.so
+            if ! docker_exec ldd build/libbpf.so | grep -q libelf; then
+                error "No reference to libelf.so in libbpf.so!"
+                exit 1
+            fi
+            info "install"
+            docker_exec make -j$((4*$(nproc))) -C src OBJDIR=../build DESTDIR=../install install
+            docker_exec rm -rf build install
+            ;;
+        CLEANUP)
+            info "Cleanup phase"
+            docker stop $CONT_NAME
+            docker rm -f $CONT_NAME
+            ;;
+        *)
+            echo >&2 "Unknown phase '$phase'"
+            exit 1
+    esac
+done

travis-ci/managers/travis_wait.bash

@@ -0,0 +1,61 @@
+# This was borrowed from https://github.com/travis-ci/travis-build/tree/master/lib/travis/build/bash
+# to get around https://github.com/travis-ci/travis-ci/issues/9979. It should probably be removed
+# as soon as Travis CI has started to provide an easy way to export the functions to bash scripts.
+
+travis_jigger() {
+  local cmd_pid="${1}"
+  shift
+  local timeout="${1}"
+  shift
+  local count=0
+
+  echo -e "\\n"
+
+  while [[ "${count}" -lt "${timeout}" ]]; do
+    count="$((count + 1))"
+    echo -ne "Still running (${count} of ${timeout}): ${*}\\r"
+    sleep 60
+  done
+
+  echo -e "\\n${ANSI_RED}Timeout (${timeout} minutes) reached. Terminating \"${*}\"${ANSI_RESET}\\n"
+  kill -9 "${cmd_pid}"
+}
+
+travis_wait() {
+  local timeout="${1}"
+
+  if [[ "${timeout}" =~ ^[0-9]+$ ]]; then
+    shift
+  else
+    timeout=20
+  fi
+
+  local cmd=("${@}")
+  local log_file="travis_wait_${$}.log"
+
+  "${cmd[@]}" &>"${log_file}" &
+  local cmd_pid="${!}"
+
+  travis_jigger "${!}" "${timeout}" "${cmd[@]}" &
+  local jigger_pid="${!}"
+  local result
+
+  {
+    set +e
+    wait "${cmd_pid}" 2>/dev/null
+    result="${?}"
+    ps -p"${jigger_pid}" &>/dev/null && kill "${jigger_pid}"
+    set -e
+  }
+
+  if [[ "${result}" -eq 0 ]]; then
+    echo -e "\\n${ANSI_GREEN}The command ${cmd[*]} exited with ${result}.${ANSI_RESET}"
+  else
+    echo -e "\\n${ANSI_RED}The command ${cmd[*]} exited with ${result}.${ANSI_RESET}"
+  fi
+
+  echo -e "\\n${ANSI_GREEN}Log:${ANSI_RESET}\\n"
+  cat "${log_file}"
+
+  return "${result}"
+}