Sorry I missed that there was a v2 already. Reposting the comments I
sent to v1 below.
On Mon, Jul 06, 2026 at 11:08:03PM -0700, Sun Jian wrote:
[...]
> @@ -5344,6 +5348,29 @@ static int __check_buffer_access(struct
> bpf_verifier_env *env,
> return -EACCES;
> }
>
> + var_off = (s64)reg->var_off.value;
> + if (check_add_overflow(var_off, (s64)off, &start)) {
> + verbose(env,
> + "%s invalid %s buffer access: off=%d, var_off=%lld\n",
> + reg_arg_name(env, argno), buf_info, off, var_off);
> + return -EACCES;
> + }
Other pointer offset checks (check_mem_region_access and
adjust_ptr_min_max_vals) seem to check var_off against BPF_MAX_VAR_OFF,
and that `off` being >= 0. It would be great if the convention can be
kept for consistency, albeit being more conservative.
>From a blind guess I would guess that compiler does not product a
pattern where `off` encoded in the instruction is negative but the net
overall offset is positive for PTR_TO_TP_BUFFER/PTR_TO_BUF, and
rejecting such pattern is fine.
r6 = *(u64 *)(r1 + 0);
r6 += 16;
r0 = *(u64 *)(r6 - 8);
One more comment below.
> +
> + if (start < 0) {
> + verbose(env,
> + "%s invalid negative %s buffer offset: off=%d,
> var_off=%lld\n",
> + reg_arg_name(env, argno), buf_info, off, var_off);
> + return -EACCES;
> + }
> +
> + if (start > U32_MAX || size < 0 ||
> + check_add_overflow((u32)start, (u32)size, access_end)) {
> + verbose(env,
> + "%s invalid %s buffer access: off=%lld, size=%d\n",
> + reg_arg_name(env, argno), buf_info, start, size);
> + return -EACCES;
> + }
If we check that var_off is within BPF_MAX_VAR_OFF and that `off` is not
negative, then `var_off + off + size` should be guaranteed to not
overflow max_tp_access, since the maximum of `off` is S16_MAX and the
maximum of `size` is 8.
> +
> return 0;
> }
>
[...]
