linux-kernel - Re: [PATCH bpf-next v9 01/23] bpf/verifier: allow all functions to read user provided context

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAP01T77SJyiDxv0A++_mNw7JZ-Mzh4B1FAM6zLiP6n75MNY0uQ@mail.gmail.com>
Date:   Fri, 2 Sep 2022 05:50:18 +0200
From:   Kumar Kartikeya Dwivedi <memxor@...il.com>
To:     Benjamin Tissoires <benjamin.tissoires@...hat.com>
Cc:     Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        Jiri Kosina <jikos@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>, Shuah Khan <shuah@...nel.org>,
        Dave Marchevsky <davemarchevsky@...com>,
        Joe Stringer <joe@...ium.io>, Jonathan Corbet <corbet@....net>,
        Tero Kristo <tero.kristo@...ux.intel.com>,
        LKML <linux-kernel@...r.kernel.org>,
        "open list:HID CORE LAYER" <linux-input@...r.kernel.org>,
        Network Development <netdev@...r.kernel.org>,
        bpf <bpf@...r.kernel.org>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@...r.kernel.org>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>
Subject: Re: [PATCH bpf-next v9 01/23] bpf/verifier: allow all functions to
 read user provided context

On Thu, 1 Sept 2022 at 18:48, Benjamin Tissoires
<benjamin.tissoires@...hat.com> wrote:
>
> [...]
> If the above is correct, then yes, it would make sense to me to have 2
> distinct functions: one to check for the args types only (does the
> function definition in the problem matches BTF), and one to check for
> its use.
> Behind the scenes, btf_check_subprog_arg_match() calls
> btf_check_func_arg_match() which is the one function with entangled
> arguments type checking and actually assessing that the values
> provided are correct.
>
> I can try to split that  btf_check_func_arg_match() into 2 distinct
> functions, though I am not sure I'll get it right.

FYI, I've already split them into separate functions in my tree
because it had become super ugly at this point with all the new
support and I refactored it to add the linked list helpers support
using kfuncs (which requires some special handling for the args), so I
think you can just leave it with a "processing_call" check in for your
series for now.

> Maybe the hack about having "processing_call" for
> btf_check_func_arg_match() only will be good enough as a first step
> towards a better solution?
>
> > And may cleanup the rest of that function ?
> > Like all of if (is_kfunc) applies only to 'calling' case.
> > Other ideas?
> >
>
> I was trying to understand the problem most of today, and the only
> other thing I could think of was "why is the assumption that
> PTR_TO_CTX is not NULL actually required?". But again, this question
> is "valid" in the function declaration part, but not in the caller
> insn part. So I think splitting btf_check_subprog_arg_match() in 2 is
> probably the best.
>
> Cheers,
> Benjamin
>