Re: [PATCH] bpf: Forbid trampoline attach for functions with variable arguments

On 5/5/21 8:45 PM, Andrii Nakryiko wrote:

On Wed, May 5, 2021 at 6:42 AM Jiri Olsa [off-list ref] wrote:

quoted

We can't currently allow to attach functions with variable arguments.
The problem is that we should save all the registers for arguments,
which is probably doable, but if caller uses more than 6 arguments,
we need stack data, which will be wrong, because of the extra stack
frame we do in bpf trampoline, so we could crash.

Also currently there's malformed trampoline code generated for such
functions at the moment as described in:
   https://lore.kernel.org/bpf/20210429212834.82621-1-jolsa@kernel.org/ (local)

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---

LGTM.

Acked-by: Andrii Nakryiko <andrii@kernel.org>

quoted

  kernel/bpf/btf.c | 13 +++++++++++++
  1 file changed, 13 insertions(+)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 0600ed325fa0..161511bb3e51 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c

@@ -5206,6 +5206,13 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
         m->ret_size = ret;

         for (i = 0; i < nargs; i++) {
+               if (i == nargs - 1 && args[i].type == 0) {
+                       bpf_log(log,
+                               "The function %s with variable args is unsupported.\n",
+                               tname);
+                       return -EINVAL;
+

(Jiri, fyi, I removed this extra newline while applying. Please scan for such
things before submitting.)

quoted

+               }
                 ret = __get_type_size(btf, args[i].type, &t);
                 if (ret < 0) {
                         bpf_log(log,

@@ -5213,6 +5220,12 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
                                 tname, i, btf_kind_str[BTF_INFO_KIND(t->info)]);
                         return -EINVAL;
                 }
+               if (ret == 0) {
+                       bpf_log(log,
+                               "The function %s has malformed void argument.\n",
+                               tname);
+                       return -EINVAL;
+               }
                 m->arg_size[i] = ret;
         }
         m->nr_args = nargs;
--

2.30.2