On Mon, Feb 13, 2023 at 8:35 PM Joanne Koong <joannelkoong@xxxxxxxxx> wrote:
>
> A bpf program calling a kfunc with a __sz-annotated arg must explicitly
> initialize the stack themselves if the pointer to the memory region is
> a pointer to the stack. This is because in the verifier, we do not
> explicitly initialize the stack space for reg type PTR_TO_STACK
> kfunc args. Thus, the verifier will reject the program with:
>
> invalid indirect read from stack
> arg#0 arg#1 memory, len pair leads to invalid memory access
>
> Alternatively, the verifier could support initializing the stack
> space on behalf of the program for KF_ARG_PTR_TO_MEM_SIZE args,
> but this has some drawbacks. For example this would not allow the
> verifier to reject a program for passing in an uninitialized
> PTR_TO_STACK for an arg that should have valid data. Another example is
> that since there's no current way in a kfunc to differentiate between
> whether the arg should be treated as uninitialized or not, additional
> check_mem_access calls would need to be called even on PTR_TO_STACKs
> that have been initialized, which is inefficient. Please note
> that non-kfuncs don't have this problem because of the MEM_UNINIT tag;
> only if the arg is tagged as MEM_UNINIT, then do we call
> check_mem_access byte-by-byte for the size of the buffer.
>
> Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx>
> ---
> Documentation/bpf/kfuncs.rst | 35 +++++++++++++++++++++++++++++++----
> 1 file changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/bpf/kfuncs.rst b/Documentation/bpf/kfuncs.rst
> index ca96ef3f6896..97497a7879d6 100644
> --- a/Documentation/bpf/kfuncs.rst
> +++ b/Documentation/bpf/kfuncs.rst
> @@ -71,10 +71,37 @@ An example is given below::
> ...
> }
>
> -Here, the verifier will treat first argument as a PTR_TO_MEM, and second
> -argument as its size. By default, without __sz annotation, the size of the type
> -of the pointer is used. Without __sz annotation, a kfunc cannot accept a void
> -pointer.
> +Here, the verifier will treat first argument (KF_ARG_PTR_TO_MEM_SIZE) as a
> +pointer to the memory region and second argument as its size. By default,
> +without __sz annotation, the size of the type of the pointer is used. Without
> +__sz annotation, a kfunc cannot accept a void pointer.
> +
> +Please note that if the memory is on the stack, the stack space must be
> +explicitly initialized by the program. For example:
> +
> +.. code-block:: c
> +
> + SEC("tc")
> + int prog(struct __sk_buff *skb)
> + {
> + char buf[8];
> +
> + bpf_memzero(buf, sizeof(buf));
> + ...
> + }
> +
> +should be
> +
> +.. code-block:: c
> +
> + SEC("tc")
> + int prog(struct __sk_buff *skb)
> + {
> + char buf[8] = {};
> +
> + bpf_memzero(buf, sizeof(buf));
Actually we might go the other way.
Instead of asking users to explicitly init things
we will allow uninit memory.
See this discussion:
https://lore.kernel.org/bpf/082fd8451321a832f334882a1872b5cee240d811.camel@xxxxxxxxx/
Eduard, is about to send those verifier patches.
In parallel we can relax __sz to accept uninit under allow_uninit_stack.
