selftests/bpf: Fix invalid pointer check in get_xlated_program()
authorEduard Zingerman <eddyz87@gmail.com>
Fri, 9 Jun 2023 22:16:37 +0000 (01:16 +0300)
committerDaniel Borkmann <daniel@iogearbox.net>
Mon, 12 Jun 2023 15:10:25 +0000 (17:10 +0200)
Dan Carpenter reported invalid check for calloc() result in
test_verifier.c:get_xlated_program():

  ./tools/testing/selftests/bpf/test_verifier.c:1365 get_xlated_program()
  warn: variable dereferenced before check 'buf' (see line 1364)

  ./tools/testing/selftests/bpf/test_verifier.c
    1363 *cnt = xlated_prog_len / buf_element_size;
    1364 *buf = calloc(*cnt, buf_element_size);
    1365 if (!buf) {

  This should be if (!*buf) {

    1366 perror("can't allocate xlated program buffer");
    1367 return -ENOMEM;

This commit refactors the get_xlated_program() to avoid using double
pointer type.

Fixes: 933ff53191eb ("selftests/bpf: specify expected instructions in test_verifier tests")
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Closes: https://lore.kernel.org/bpf/ZH7u0hEGVB4MjGZq@moroto/
Link: https://lore.kernel.org/bpf/20230609221637.2631800-1-eddyz87@gmail.com
tools/testing/selftests/bpf/test_verifier.c

index 71704a3..31f1c93 100644 (file)
@@ -1341,45 +1341,46 @@ static bool cmp_str_seq(const char *log, const char *exp)
        return true;
 }
 
-static int get_xlated_program(int fd_prog, struct bpf_insn **buf, int *cnt)
+static struct bpf_insn *get_xlated_program(int fd_prog, int *cnt)
 {
+       __u32 buf_element_size = sizeof(struct bpf_insn);
        struct bpf_prog_info info = {};
        __u32 info_len = sizeof(info);
        __u32 xlated_prog_len;
-       __u32 buf_element_size = sizeof(struct bpf_insn);
+       struct bpf_insn *buf;
 
        if (bpf_prog_get_info_by_fd(fd_prog, &info, &info_len)) {
                perror("bpf_prog_get_info_by_fd failed");
-               return -1;
+               return NULL;
        }
 
        xlated_prog_len = info.xlated_prog_len;
        if (xlated_prog_len % buf_element_size) {
                printf("Program length %d is not multiple of %d\n",
                       xlated_prog_len, buf_element_size);
-               return -1;
+               return NULL;
        }
 
        *cnt = xlated_prog_len / buf_element_size;
-       *buf = calloc(*cnt, buf_element_size);
+       buf = calloc(*cnt, buf_element_size);
        if (!buf) {
                perror("can't allocate xlated program buffer");
-               return -ENOMEM;
+               return NULL;
        }
 
        bzero(&info, sizeof(info));
        info.xlated_prog_len = xlated_prog_len;
-       info.xlated_prog_insns = (__u64)(unsigned long)*buf;
+       info.xlated_prog_insns = (__u64)(unsigned long)buf;
        if (bpf_prog_get_info_by_fd(fd_prog, &info, &info_len)) {
                perror("second bpf_prog_get_info_by_fd failed");
                goto out_free_buf;
        }
 
-       return 0;
+       return buf;
 
 out_free_buf:
-       free(*buf);
-       return -1;
+       free(buf);
+       return NULL;
 }
 
 static bool is_null_insn(struct bpf_insn *insn)
@@ -1512,7 +1513,8 @@ static bool check_xlated_program(struct bpf_test *test, int fd_prog)
        if (!check_expected && !check_unexpected)
                goto out;
 
-       if (get_xlated_program(fd_prog, &buf, &cnt)) {
+       buf = get_xlated_program(fd_prog, &cnt);
+       if (!buf) {
                printf("FAIL: can't get xlated program\n");
                result = false;
                goto out;