linux-kernel - Re: [PATCH bpf-next v6 05/23] bpf/verifier: allow kfunc to return an allocated mem

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] [day] [month] [year] [list]

Date:   Tue, 19 Jul 2022 09:05:07 -0700
From:   Yonghong Song <yhs@...com>
To:     Benjamin Tissoires <benjamin.tissoires@...hat.com>
Cc:     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>,
        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>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@...r.kernel.org>,
        Linux Doc Mailing List <linux-doc@...r.kernel.org>
Subject: Re: [PATCH bpf-next v6 05/23] bpf/verifier: allow kfunc to return an
 allocated mem



On 7/18/22 7:36 AM, Benjamin Tissoires wrote:
> On Sat, Jul 16, 2022 at 6:29 AM Yonghong Song <yhs@...com> wrote:
>>
>>
>>
>> On 7/12/22 7:58 AM, Benjamin Tissoires wrote:
>>> When a kfunc is not returning a pointer to a struct but to a plain type,
>>> we can consider it is a valid allocated memory assuming that:
>>> - one of the arguments is either called rdonly_buf_size or
>>>     rdwr_buf_size
>>> - and this argument is a const from the caller point of view
>>>
>>> We can then use this parameter as the size of the allocated memory.
>>>
>>> The memory is either read-only or read-write based on the name
>>> of the size parameter.
>>
>> If I understand correctly, this permits a kfunc like
>>      int *kfunc(..., int rdonly_buf_size);
>>      ...
>>      int *p = kfunc(..., 20);
>> so the 'p' points to a memory buffer with size 20.
> 
> Yes, exactly.
> 
>>
>> This looks like a strange interface although probably there
>> is a valid reason for this as I didn't participated in
>> earlier discussions.
> 
> Well, the point is I need to be able to access a memory region that
> was allocated dynamically. For drivers, the incoming data can not
> usually be bound to a static value, and so we can not have the data
> statically defined in the matching struct.
> So this allows defining a kfunc to return any memory properly
> allocated and owned by the device.

Okay, thanks for explanation.

> 
>>
>>>
>>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@...hat.com>
>>>
>>> ---
>>>
>>> changes in v6:
>>> - code review from Kartikeya:
>>>     - remove comment change that had no reasons to be
>>>     - remove handling of PTR_TO_MEM with kfunc releases
>>>     - introduce struct bpf_kfunc_arg_meta
>>>     - do rdonly/rdwr_buf_size check in btf_check_kfunc_arg_match
>>>     - reverted most of the changes in verifier.c
>>>     - make sure kfunc acquire is using a struct pointer, not just a plain
>>>       pointer
>>>     - also forward ref_obj_id to PTR_TO_MEM in kfunc to not use after free
>>>       the allocated memory
>>>
>>> changes in v5:
>>> - updated PTR_TO_MEM comment in btf.c to match upstream
>>> - make it read-only or read-write based on the name of size
>>>
>>> new in v4
>>> ---
>>>    include/linux/bpf.h   | 10 ++++++-
>>>    include/linux/btf.h   | 12 ++++++++
>>>    kernel/bpf/btf.c      | 67 ++++++++++++++++++++++++++++++++++++++++---
>>>    kernel/bpf/verifier.c | 49 +++++++++++++++++++++++--------
>>>    4 files changed, 121 insertions(+), 17 deletions(-)
>>>
[...]