Thread (13 messages) 13 messages, 3 authors, 2021-02-07

Re: [PATCH bpf-next v2 1/5] bpf: allow variable-offset stack access

From: Andrei Matei <hidden>
Date: 2021-02-07 01:12:30

quoted
+/* Check that the stack access at 'regno + off' falls within the maximum stack
+ * bounds.
+ *
+ * 'off' includes `regno->offset`, but not its dynamic part (if any).
+ */
+static int check_stack_access_within_bounds(
+             struct bpf_verifier_env *env,
+             int regno, int off, int access_size,
+             enum stack_access_src src, enum bpf_access_type type)
+{
+     struct bpf_reg_state *regs = cur_regs(env);
+     struct bpf_reg_state *reg = regs + regno;
+     struct bpf_func_state *state = func(env, reg);
+     int min_off, max_off;
+     int err;
+     char *err_extra;
+
+     if (src == ACCESS_HELPER)
the ACCESS_HELPER|DIRECT enum should probably be moved right before this function.
It's not used earlier, I think, and it made the reviewing a bit harder than could have been.
It is, unfortunately. ACCESS_DIRECT is used close to where it is
defined, in check_stack_read_var_off().
quoted
+             /* We don't know if helpers are reading or writing (or both). */
+             err_extra = " indirect access to";
+     else if (type == BPF_READ)
+             err_extra = " read from";
+     else
+             err_extra = " write to";
Thanks for improving verifier errors.
quoted
+
+     if (tnum_is_const(reg->var_off)) {
+             min_off = reg->var_off.value + off;
+             if (access_size > 0)
+                     max_off = min_off + access_size - 1;
+             else
+                     max_off = min_off;
+     } else {
+             if (reg->smax_value >= BPF_MAX_VAR_OFF ||
+                 reg->smax_value <= -BPF_MAX_VAR_OFF) {
hmm. are you sure about smax in both conditions? looks like typo?
This is how it used to be before this patch btw, but I think you're
right. It looks like the second one should have been smin_value.
Fixing here.
Existing code: https://github.com/torvalds/linux/blob/1e0d27fce010b0a4a9e595506b6ede75934c31be/kernel/bpf/verifier.c#L3721
quoted
+                     verbose(env, "invalid unbounded variable-offset%s stack R%d\n",
+                             err_extra, regno);
+                     return -EACCES;
+             }
+             min_off = reg->smin_value + off;
+             if (access_size > 0)
+                     max_off = reg->smax_value + off + access_size - 1;
+             else
+                     max_off = min_off;
+     }
The rest looks good.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help