Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230406004018.1439952-1-drosen@google.com> <20230406004018.1439952-2-drosen@google.com>
In-Reply-To: <20230406004018.1439952-2-drosen@google.com>
From:   Andrii Nakryiko <andrii.nakryiko@gmail.com>
Date:   Thu, 6 Apr 2023 13:55:23 -0700
Message-ID: <CAEf4BzbyX3i6k5eL6D-5enU+u58nVn_fK28zNBJ4w_Vm-+RiMQ@mail.gmail.com>
Subject: Re: [PATCH 1/3] bpf: verifier: Accept dynptr mem as mem in helpers
To:     Daniel Rosenberg <drosen@google.com>
Cc:     bpf@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        John Fastabend <john.fastabend@gmail.com>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <martin.lau@linux.dev>,
        Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>,
        KP Singh <kpsingh@kernel.org>,
        Stanislav Fomichev <sdf@google.com>,
        Hao Luo <haoluo@google.com>, Jiri Olsa <jolsa@kernel.org>,
        Shuah Khan <shuah@kernel.org>,
        Jonathan Corbet <corbet@lwn.net>,
        Joanne Koong <joannelkoong@gmail.com>,
        Mykola Lysenko <mykolal@fb.com>, linux-kernel@vger.kernel.org,
        linux-kselftest@vger.kernel.org, kernel-team@android.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Apr 5, 2023 at 5:40=E2=80=AFPM Daniel Rosenberg <drosen@google.com>=
 wrote:
>
> This allows using memory retrieved from dynptrs with helper functions
> that accept ARG_PTR_TO_MEM. For instance, results from bpf_dynptr_data
> can be passed along to bpf_strncmp.
>
> Signed-off-by: Daniel Rosenberg <drosen@google.com>
> ---

I think patches like this should be targeted against bpf-next, so for
next revision please send them against bpf-next tree.

>  kernel/bpf/verifier.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 56f569811f70..20beab52812a 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -7164,12 +7164,16 @@ static int check_reg_type(struct bpf_verifier_env=
 *env, u32 regno,
>          * ARG_PTR_TO_MEM + MAYBE_NULL is compatible with PTR_TO_MEM and =
PTR_TO_MEM + MAYBE_NULL,
>          * but ARG_PTR_TO_MEM is compatible only with PTR_TO_MEM but NOT =
with PTR_TO_MEM + MAYBE_NULL
>          *
> +        * ARG_PTR_TO_MEM is compatible with PTR_TO_MEM that is tagged wi=
th a dynptr type.
> +        *
>          * Therefore we fold these flags depending on the arg_type before=
 comparison.
>          */
>         if (arg_type & MEM_RDONLY)
>                 type &=3D ~MEM_RDONLY;
>         if (arg_type & PTR_MAYBE_NULL)
>                 type &=3D ~PTR_MAYBE_NULL;
> +       if (base_type(arg_type) =3D=3D ARG_PTR_TO_MEM)
> +               type &=3D ~DYNPTR_TYPE_FLAG_MASK;

Something feels off here. Can you paste a bit of verifier log for the
failure you were getting. And let's have a selftest for this situation
as well.

ARG_PTR_TO_MEM shouldn't be qualified with the DYNPTR_TYPE flag, it's
just memory, there is no need to know what type of dynptr it was
derived from. So if that happens, the problem is somewhere else. Let's
root cause and fix that. Having a selftest that demonstrates the
problem will help with that.


>
>         if (meta->func_id =3D=3D BPF_FUNC_kptr_xchg && type & MEM_ALLOC)
>                 type &=3D ~MEM_ALLOC;
> --
> 2.40.0.577.gac1e443424-goog
>