Re: [PATCH] bpf: Ensure BPF programs testing skb context initialization

[Alexei Starovoitov]

On Wed, Jul 10, 2024 at 4:58 AM Michal Switala
<michal.switala@xxxxxxxxxxxx> wrote:
>
> This commit addresses an issue where a netdevice was found to be uninitialized.
> To mitigate this case, the change ensures that BPF programs designed to test
> skb context initialization thoroughly verify the availability of a fully
> initialized context before execution.The root cause of a NULL ctx stems from
> the initialization process in bpf_ctx_init(). This function returns NULL if
> the user initializes the bpf_attr variables ctx_in and ctx_out with invalid
> pointers or sets them to NULL. These variables are directly controlled by user
> input, and if both are NULL, the context cannot be initialized, resulting in a
> NULL ctx.
>
> Reported-by: syzbot+cca39e6e84a367a7e6f6@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=cca39e6e84a367a7e6f6
> Link: https://lore.kernel.org/all/000000000000b95d41061cbf302a@xxxxxxxxxx/

Something doesn't add up.
This syzbot report is about:

dev_map_enqueue+0x31/0x3e0 kernel/bpf/devmap.c:539
__xdp_do_redirect_frame net/core/filter.c:4397 [inline]
bpf_prog_test_run_xdp

while you're fixing bpf_prog_test_run_skb ?

pw-bot: cr

> Signed-off-by: Michal Switala <michal.switala@xxxxxxxxxxxx>
> ---
>  net/bpf/test_run.c | 30 +++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 36ae54f57bf5..8b2efcee059f 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -970,7 +970,7 @@ static struct proto bpf_dummy_proto = {
>  int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>                           union bpf_attr __user *uattr)
>  {
> -       bool is_l2 = false, is_direct_pkt_access = false;
> +       bool is_l2 = false, is_direct_pkt_access = false, ctx_needed = false;
>         struct net *net = current->nsproxy->net_ns;
>         struct net_device *dev = net->loopback_dev;
>         u32 size = kattr->test.data_size_in;
> @@ -998,6 +998,34 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>                 return PTR_ERR(ctx);
>         }
>
> +       switch (prog->type) {
> +       case BPF_PROG_TYPE_SOCKET_FILTER:
> +       case BPF_PROG_TYPE_SCHED_CLS:
> +       case BPF_PROG_TYPE_SCHED_ACT:
> +       case BPF_PROG_TYPE_XDP:
> +       case BPF_PROG_TYPE_CGROUP_SKB:
> +       case BPF_PROG_TYPE_CGROUP_SOCK:
> +       case BPF_PROG_TYPE_SOCK_OPS:
> +       case BPF_PROG_TYPE_SK_SKB:
> +       case BPF_PROG_TYPE_SK_MSG:
> +       case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
> +       case BPF_PROG_TYPE_LWT_SEG6LOCAL:
> +       case BPF_PROG_TYPE_SK_REUSEPORT:
> +       case BPF_PROG_TYPE_NETFILTER:
> +       case BPF_PROG_TYPE_LWT_IN:
> +       case BPF_PROG_TYPE_LWT_OUT:
> +       case BPF_PROG_TYPE_LWT_XMIT:
> +               ctx_needed = true;
> +               break;
> +       default:
> +               break;
> +       }
> +
> +       if (!ctx && ctx_needed) {
> +               kfree(data);
> +               return -EINVAL;
> +       }
> +
>         switch (prog->type) {
>         case BPF_PROG_TYPE_SCHED_CLS:
>         case BPF_PROG_TYPE_SCHED_ACT:
> --
> 2.43.0
>
>