bpf: Simplify internal verifier log interface

Simplify internal verifier log API down to bpf_vlog_init() and
bpf_vlog_finalize(). The former handles input arguments validation in
one place and makes it easier to change it. The latter subsumes -ENOSPC
(truncation) and -EFAULT handling and simplifies both caller's code
(bpf_check() and btf_parse()).

For btf_parse(), this patch also makes sure that verifier log
finalization happens even if there is some error condition during BTF
verification process prior to normal finalization step.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Lorenz Bauer <lmb@isovalent.com>
Link: https://lore.kernel.org/bpf/20230406234205.323208-14-andrii@kernel.org

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 98d2eb382dbb79ea1981ec066c503a5d944d58c0..f03852b89d289cd5f82b6a079df48ec6d6c79180 100644 (file)
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -518,13 +518,6 @@ struct bpf_verifier_log {
 #define BPF_LOG_MIN_ALIGNMENT 8U
 #define BPF_LOG_ALIGNMENT 40U
 
-static inline bool bpf_verifier_log_full(const struct bpf_verifier_log *log)
-{
-       if (log->level & BPF_LOG_FIXED)
-               return log->end_pos >= log->len_total;
-       return false;
-}
-
 static inline bool bpf_verifier_log_needed(const struct bpf_verifier_log *log)
 {
        return log && log->level;
@@ -612,16 +605,16 @@ struct bpf_verifier_env {
        char type_str_buf[TYPE_STR_BUF_LEN];
 };
 
-bool bpf_verifier_log_attr_valid(const struct bpf_verifier_log *log);
 __printf(2, 0) void bpf_verifier_vlog(struct bpf_verifier_log *log,
                                      const char *fmt, va_list args);
 __printf(2, 3) void bpf_verifier_log_write(struct bpf_verifier_env *env,
                                           const char *fmt, ...);
 __printf(2, 3) void bpf_log(struct bpf_verifier_log *log,
                            const char *fmt, ...);
+int bpf_vlog_init(struct bpf_verifier_log *log, u32 log_level,
+                 char __user *log_buf, u32 log_size);
 void bpf_vlog_reset(struct bpf_verifier_log *log, u64 new_pos);
-void bpf_vlog_finalize(struct bpf_verifier_log *log);
-bool bpf_vlog_truncated(const struct bpf_verifier_log *log);
+int bpf_vlog_finalize(struct bpf_verifier_log *log, u32 *log_size_actual);
 
 static inline struct bpf_func_state *cur_func(struct bpf_verifier_env *env)
 {

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 0748cf4b8ab68c3a012dab370e565ede5c42c869..ffc31a1c84afb1dad441b79a3c2cb16079660836 100644 (file)
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -5504,16 +5504,30 @@ static int btf_check_type_tags(struct btf_verifier_env *env,
        return 0;
 }
 
+static int finalize_log(struct bpf_verifier_log *log, bpfptr_t uattr, u32 uattr_size)
+{
+       u32 log_true_size;
+       int err;
+
+       err = bpf_vlog_finalize(log, &log_true_size);
+
+       if (uattr_size >= offsetofend(union bpf_attr, btf_log_true_size) &&
+           copy_to_bpfptr_offset(uattr, offsetof(union bpf_attr, btf_log_true_size),
+                                 &log_true_size, sizeof(log_true_size)))
+               err = -EFAULT;
+
+       return err;
+}
+
 static struct btf *btf_parse(const union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size)
 {
        bpfptr_t btf_data = make_bpfptr(attr->btf, uattr.is_kernel);
        char __user *log_ubuf = u64_to_user_ptr(attr->btf_log_buf);
        struct btf_struct_metas *struct_meta_tab;
        struct btf_verifier_env *env = NULL;
-       struct bpf_verifier_log *log;
        struct btf *btf = NULL;
        u8 *data;
-       int err;
+       int err, ret;
 
        if (attr->btf_size > BTF_MAX_SIZE)
                return ERR_PTR(-E2BIG);
@@ -5522,21 +5536,13 @@ static struct btf *btf_parse(const union bpf_attr *attr, bpfptr_t uattr, u32 uat
        if (!env)
                return ERR_PTR(-ENOMEM);
 
-       log = &env->log;
-       if (attr->btf_log_level || log_ubuf || attr->btf_log_size) {
-               /* user requested verbose verifier output
-                * and supplied buffer to store the verification trace
-                */
-               log->level = attr->btf_log_level;
-               log->ubuf = log_ubuf;
-               log->len_total = attr->btf_log_size;
-
-               /* log attributes have to be sane */
-               if (!bpf_verifier_log_attr_valid(log)) {
-                       err = -EINVAL;
-                       goto errout;
-               }
-       }
+       /* user could have requested verbose verifier output
+        * and supplied buffer to store the verification trace
+        */
+       err = bpf_vlog_init(&env->log, attr->btf_log_level,
+                           log_ubuf, attr->btf_log_size);
+       if (err)
+               goto errout_free;
 
        btf = kzalloc(sizeof(*btf), GFP_KERNEL | __GFP_NOWARN);
        if (!btf) {
@@ -5577,7 +5583,7 @@ static struct btf *btf_parse(const union bpf_attr *attr, bpfptr_t uattr, u32 uat
        if (err)
                goto errout;
 
-       struct_meta_tab = btf_parse_struct_metas(log, btf);
+       struct_meta_tab = btf_parse_struct_metas(&env->log, btf);
        if (IS_ERR(struct_meta_tab)) {
                err = PTR_ERR(struct_meta_tab);
                goto errout;
@@ -5594,21 +5600,9 @@ static struct btf *btf_parse(const union bpf_attr *attr, bpfptr_t uattr, u32 uat
                }
        }
 
-       bpf_vlog_finalize(log);
-       if (uattr_size >= offsetofend(union bpf_attr, btf_log_true_size) &&
-           copy_to_bpfptr_offset(uattr, offsetof(union bpf_attr, btf_log_true_size),
-                                 &log->len_max, sizeof(log->len_max))) {
-               err = -EFAULT;
-               goto errout_meta;
-       }
-       if (bpf_vlog_truncated(log)) {
-               err = -ENOSPC;
-               goto errout_meta;
-       }
-       if (log->level && log->level != BPF_LOG_KERNEL && !log->ubuf) {
-               err = -EFAULT;
-               goto errout_meta;
-       }
+       err = finalize_log(&env->log, uattr, uattr_size);
+       if (err)
+               goto errout_free;
 
        btf_verifier_env_free(env);
        refcount_set(&btf->refcnt, 1);
@@ -5617,6 +5611,11 @@ static struct btf *btf_parse(const union bpf_attr *attr, bpfptr_t uattr, u32 uat
 errout_meta:
        btf_free_struct_meta_tab(btf);
 errout:
+       /* overwrite err with -ENOSPC or -EFAULT */
+       ret = finalize_log(&env->log, uattr, uattr_size);
+       if (ret)
+               err = ret;
+errout_free:
        btf_verifier_env_free(env);
        if (btf)
                btf_free(btf);

diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c
index 47bea2fad6fe2f1905b43c7a1e602fd7702b00ba..1fae2c5d7ae49889768f055832512345b355ef73 100644 (file)
--- a/kernel/bpf/log.c
+++ b/kernel/bpf/log.c
@@ -10,12 +10,26 @@
 #include <linux/bpf_verifier.h>
 #include <linux/math64.h>
 
-bool bpf_verifier_log_attr_valid(const struct bpf_verifier_log *log)
+static bool bpf_verifier_log_attr_valid(const struct bpf_verifier_log *log)
 {
        return log->len_total > 0 && log->len_total <= UINT_MAX >> 2 &&
               log->level && log->ubuf && !(log->level & ~BPF_LOG_MASK);
 }
 
+int bpf_vlog_init(struct bpf_verifier_log *log, u32 log_level,
+                 char __user *log_buf, u32 log_size)
+{
+       log->level = log_level;
+       log->ubuf = log_buf;
+       log->len_total = log_size;
+
+       /* log attributes have to be sane */
+       if (!bpf_verifier_log_attr_valid(log))
+               return -EINVAL;
+
+       return 0;
+}
+
 static void bpf_vlog_update_len_max(struct bpf_verifier_log *log, u32 add_len)
 {
        /* add_len includes terminal \0, so no need for +1. */
@@ -197,24 +211,25 @@ static int bpf_vlog_reverse_ubuf(struct bpf_verifier_log *log, int start, int en
        return 0;
 }
 
-bool bpf_vlog_truncated(const struct bpf_verifier_log *log)
+static bool bpf_vlog_truncated(const struct bpf_verifier_log *log)
 {
        return log->len_max > log->len_total;
 }
 
-void bpf_vlog_finalize(struct bpf_verifier_log *log)
+int bpf_vlog_finalize(struct bpf_verifier_log *log, u32 *log_size_actual)
 {
        u32 sublen;
        int err;
 
-       if (!log || !log->level || !log->ubuf)
-               return;
-       if ((log->level & BPF_LOG_FIXED) || log->level == BPF_LOG_KERNEL)
-               return;
+       *log_size_actual = 0;
+       if (!log || log->level == 0 || log->level == BPF_LOG_KERNEL)
+               return 0;
 
+       if (!log->ubuf)
+               goto skip_log_rotate;
        /* If we never truncated log, there is nothing to move around. */
-       if (log->start_pos == 0)
-               return;
+       if ((log->level & BPF_LOG_FIXED) || log->start_pos == 0)
+               goto skip_log_rotate;
 
        /* Otherwise we need to rotate log contents to make it start from the
         * buffer beginning and be a continuous zero-terminated string. Note
@@ -257,6 +272,21 @@ void bpf_vlog_finalize(struct bpf_verifier_log *log)
        err = err ?: bpf_vlog_reverse_ubuf(log, sublen, log->len_total);
        if (err)
                log->ubuf = NULL;
+
+skip_log_rotate:
+       *log_size_actual = log->len_max;
+
+       /* properly initialized log has either both ubuf!=NULL and len_total>0
+        * or ubuf==NULL and len_total==0, so if this condition doesn't hold,
+        * we got a fault somewhere along the way, so report it back
+        */
+       if (!!log->ubuf != !!log->len_total)
+               return -EFAULT;
+
+       if (bpf_vlog_truncated(log))
+               return -ENOSPC;
+
+       return 0;
 }
 
 /* log_level controls verbosity level of eBPF verifier.

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 308e7abeb97972a0ac6455745a6931df156f2644..d6db6de3e9eaa8bb1f561e4e46029a51e1a63be8 100644 (file)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -18698,8 +18698,8 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u3
 {
        u64 start_time = ktime_get_ns();
        struct bpf_verifier_env *env;
-       struct bpf_verifier_log *log;
-       int i, len, ret = -EINVAL;
+       int i, len, ret = -EINVAL, err;
+       u32 log_true_size;
        bool is_priv;
 
        /* no program is valid */
@@ -18712,7 +18712,6 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u3
        env = kzalloc(sizeof(struct bpf_verifier_env), GFP_KERNEL);
        if (!env)
                return -ENOMEM;
-       log = &env->log;
 
        len = (*prog)->len;
        env->insn_aux_data =
@@ -18733,20 +18732,14 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u3
        if (!is_priv)
                mutex_lock(&bpf_verifier_lock);
 
-       if (attr->log_level || attr->log_buf || attr->log_size) {
-               /* user requested verbose verifier output
-                * and supplied buffer to store the verification trace
-                */
-               log->level = attr->log_level;
-               log->ubuf = (char __user *) (unsigned long) attr->log_buf;
-               log->len_total = attr->log_size;
-
-               /* log attributes have to be sane */
-               if (!bpf_verifier_log_attr_valid(log)) {
-                       ret = -EINVAL;
-                       goto err_unlock;
-               }
-       }
+       /* user could have requested verbose verifier output
+        * and supplied buffer to store the verification trace
+        */
+       ret = bpf_vlog_init(&env->log, attr->log_level,
+                           (char __user *) (unsigned long) attr->log_buf,
+                           attr->log_size);
+       if (ret)
+               goto err_unlock;
 
        mark_verifier_state_clean(env);
 
@@ -18860,17 +18853,17 @@ skip_full_check:
        print_verification_stats(env);
        env->prog->aux->verified_insns = env->insn_processed;
 
-       bpf_vlog_finalize(log);
+       /* preserve original error even if log finalization is successful */
+       err = bpf_vlog_finalize(&env->log, &log_true_size);
+       if (err)
+               ret = err;
+
        if (uattr_size >= offsetofend(union bpf_attr, log_true_size) &&
            copy_to_bpfptr_offset(uattr, offsetof(union bpf_attr, log_true_size),
-                                 &log->len_max, sizeof(log->len_max))) {
+                                 &log_true_size, sizeof(log_true_size))) {
                ret = -EFAULT;
                goto err_release_maps;
        }
-       if (bpf_vlog_truncated(log))
-               ret = -ENOSPC;
-       if (log->level && log->level != BPF_LOG_KERNEL && !log->ubuf)
-               ret = -EFAULT;
 
        if (ret)
                goto err_release_maps;