linux-kernel - Re: [PATCH v9 06/10] bpf: Add bpf_lookup_*_key() and bpf_key

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]

Date:   Wed, 10 Aug 2022 16:29:50 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Roberto Sassu <roberto.sassu@...wei.com>,
        "ast@...nel.org" <ast@...nel.org>,
        "andrii@...nel.org" <andrii@...nel.org>,
        "martin.lau@...ux.dev" <martin.lau@...ux.dev>,
        "song@...nel.org" <song@...nel.org>, "yhs@...com" <yhs@...com>,
        "john.fastabend@...il.com" <john.fastabend@...il.com>,
        "kpsingh@...nel.org" <kpsingh@...nel.org>,
        "sdf@...gle.com" <sdf@...gle.com>,
        "haoluo@...gle.com" <haoluo@...gle.com>,
        "jolsa@...nel.org" <jolsa@...nel.org>,
        "corbet@....net" <corbet@....net>,
        "dhowells@...hat.com" <dhowells@...hat.com>,
        "jarkko@...nel.org" <jarkko@...nel.org>,
        "rostedt@...dmis.org" <rostedt@...dmis.org>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "paul@...l-moore.com" <paul@...l-moore.com>,
        "jmorris@...ei.org" <jmorris@...ei.org>,
        "serge@...lyn.com" <serge@...lyn.com>,
        "shuah@...nel.org" <shuah@...nel.org>
Cc:     "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "keyrings@...r.kernel.org" <keyrings@...r.kernel.org>,
        "linux-security-module@...r.kernel.org" 
        <linux-security-module@...r.kernel.org>,
        "linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v9 06/10] bpf: Add bpf_lookup_*_key() and bpf_key_put()
 kfuncs

[...]
>>> +noinline __weak struct bpf_key *bpf_lookup_user_key(u32 serial, u64 flags)
>>
>> Why the need for noinline and the __weak here and below? (If indeed needed
>> this
>> should probably be explained in the commit desc.)
> 
> Oh, I took from v3 of KP's patch set. It is gone in v5. Will do
> the same as well.
> 
>>> +{
>>> +	key_ref_t key_ref;
>>> +	struct bpf_key *bkey;
>>> +
>>> +	/* Keep in sync with include/linux/key.h. */
>>> +	if (flags > (KEY_LOOKUP_PARTIAL << 1) - 1)
>>
>> Can't we just simplify and test flags &
>> ~(KEY_LOOKUP_CREATE|KEY_LOOKUP_PARTIAL)?
> 
> I thought as if we have many flags.

I'd keep it simple for now, and if the actual need comes this can still be changed.

>>> +		return NULL;
>>> +
>>> +	/* Permission check is deferred until actual kfunc using the key. */
>>> +	key_ref = lookup_user_key(serial, flags, KEY_DEFER_PERM_CHECK);
>>> +	if (IS_ERR(key_ref))
>>> +		return NULL;
>>> +
>>> +	bkey = kmalloc(sizeof(*bkey), GFP_KERNEL);
>>> +	if (!bkey) {
>>> +		key_put(key_ref_to_ptr(key_ref));
>>> +		return bkey;
>>
>> nit: just return NULL probably cleaner
> 
> Ok.