On Mon, 21 Oct 2024 at 17:28, Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote:
>
> Lonial reported an issue in the BPF verifier where check_mem_size_reg()
> has the following code:
>
> if (!tnum_is_const(reg->var_off))
> /* For unprivileged variable accesses, disable raw
> * mode so that the program is required to
> * initialize all the memory that the helper could
> * just partially fill up.
> */
> meta = NULL;
>
> This means that writes are not checked when the register containing the
> size of the passed buffer has not a fixed size. Through this bug, a BPF
> program can write to a map which is marked as read-only, for example,
> .rodata global maps.
>
> The problem is that MEM_UNINIT's initial meaning that "the passed buffer
> to the BPF helper does not need to be initialized" which was added back
> in commit 435faee1aae9 ("bpf, verifier: add ARG_PTR_TO_RAW_STACK type")
> got overloaded over time with "the passed buffer is being written to".
>
> The problem however is that checks such as the above which were added later
> via 06c1c049721a ("bpf: allow helpers access to variable memory") set meta
> to NULL in order force the user to always initialize the passed buffer to
> the helper. Due to the current double meaning of MEM_UNINIT, this bypasses
> verifier write checks to the memory (not boundary checks though) and only
> assumes the latter memory is read instead.
>
> Fix this by reverting MEM_UNINIT back to its original meaning, and having
> MEM_WRITE as an annotation to BPF helpers in order to then trigger the
> BPF verifier checks for writing to memory.
>
> Some notes: check_arg_pair_ok() ensures that for ARG_CONST_SIZE{,_OR_ZERO}
> we can access fn->arg_type[arg - 1] since it must contain a preceding
> ARG_PTR_TO_MEM. For check_mem_reg() the meta argument can be removed
> altogether since we do check both BPF_READ and BPF_WRITE. Same for the
> equivalent check_kfunc_mem_size_reg().
>
> Fixes: 7b3552d3f9f6 ("bpf: Reject writes for PTR_TO_MAP_KEY in check_helper_mem_access")
> Fixes: 97e6d7dab1ca ("bpf: Check PTR_TO_MEM | MEM_RDONLY in check_helper_mem_access")
> Fixes: 15baa55ff5b0 ("bpf/verifier: allow all functions to read user provided context")
> Reported-by: Lonial Con <kongln9170@xxxxxxxxx>
> Signed-off-by: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
> ---
Acked-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
