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;
Subject: Re: [PATCH v9 06/10] bpf: Add bpf_lookup_*_key() and bpf_key_put()
 kfuncs
To:     Roberto Sassu <roberto.sassu@huawei.com>,
        "ast@kernel.org" <ast@kernel.org>,
        "andrii@kernel.org" <andrii@kernel.org>,
        "martin.lau@linux.dev" <martin.lau@linux.dev>,
        "song@kernel.org" <song@kernel.org>, "yhs@fb.com" <yhs@fb.com>,
        "john.fastabend@gmail.com" <john.fastabend@gmail.com>,
        "kpsingh@kernel.org" <kpsingh@kernel.org>,
        "sdf@google.com" <sdf@google.com>,
        "haoluo@google.com" <haoluo@google.com>,
        "jolsa@kernel.org" <jolsa@kernel.org>,
        "corbet@lwn.net" <corbet@lwn.net>,
        "dhowells@redhat.com" <dhowells@redhat.com>,
        "jarkko@kernel.org" <jarkko@kernel.org>,
        "rostedt@goodmis.org" <rostedt@goodmis.org>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "paul@paul-moore.com" <paul@paul-moore.com>,
        "jmorris@namei.org" <jmorris@namei.org>,
        "serge@hallyn.com" <serge@hallyn.com>,
        "shuah@kernel.org" <shuah@kernel.org>
Cc:     "bpf@vger.kernel.org" <bpf@vger.kernel.org>,
        "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
        "keyrings@vger.kernel.org" <keyrings@vger.kernel.org>,
        "linux-security-module@vger.kernel.org" 
        <linux-security-module@vger.kernel.org>,
        "linux-kselftest@vger.kernel.org" <linux-kselftest@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
References: <20220809134603.1769279-1-roberto.sassu@huawei.com>
 <20220809134603.1769279-7-roberto.sassu@huawei.com>
 <2b1d62ad-af4b-4694-ecc8-639fbd821a05@iogearbox.net>
 <ecc84dd76f4c429bb55e6bd8fece376b@huawei.com>
From:   Daniel Borkmann <daniel@iogearbox.net>
Message-ID: <d0c0cda2-9a79-35d7-39cf-aad8a12642d9@iogearbox.net>
Date:   Wed, 10 Aug 2022 16:29:50 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.7.2
MIME-Version: 1.0
In-Reply-To: <ecc84dd76f4c429bb55e6bd8fece376b@huawei.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

[...]
>>> +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.