Re: [PATCH v17 11/12] selftests/bpf: Add test for bpf_verify_pkcs7_signature() kfunc

From: Yonghong Song <yhs@fb.com>
To: Roberto Sassu <roberto.sassu@huaweicloud.com>,
	KP Singh <kpsingh@kernel.org>
Cc: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
	martin.lau@linux.dev, song@kernel.org, john.fastabend@gmail.com,
	sdf@google.com, haoluo@google.com, jolsa@kernel.org,
	mykolal@fb.com, dhowells@redhat.com, jarkko@kernel.org,
	rostedt@goodmis.org, mingo@redhat.com, paul@paul-moore.com,
	jmorris@namei.org, serge@hallyn.com, shuah@kernel.org,
	bpf@vger.kernel.org, keyrings@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
	deso@posteo.net, memxor@gmail.com,
	Roberto Sassu <roberto.sassu@huawei.com>,
	Joanne Koong <joannelkoong@gmail.com>
Subject: Re: [PATCH v17 11/12] selftests/bpf: Add test for bpf_verify_pkcs7_signature() kfunc
Date: Mon, 19 Sep 2022 22:26:51 -0700	[thread overview]
Message-ID: <eae58ded-7d6b-bf26-d302-391e3746f7ac@fb.com> (raw)
In-Reply-To: <64e8ef59363bcd0f314d4e1eb7483f4dd2b7dbcf.camel@huaweicloud.com>

On 9/19/22 6:09 AM, Roberto Sassu wrote:
> On Mon, 2022-09-19 at 13:17 +0200, Roberto Sassu wrote:
>> On Thu, 2022-09-15 at 17:11 +0100, KP Singh wrote:
>>> On Fri, Sep 9, 2022 at 1:10 PM Roberto Sassu
>>> <roberto.sassu@huaweicloud.com> wrote:
>>>> From: Roberto Sassu <roberto.sassu@huawei.com>
>>>>
>>>
>>> [...]
>>>
>>>> +}
>>>> diff --git
>>>> a/tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c
>>>> b/tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c
>>>> new file mode 100644
>>>> index 000000000000..4ceab545d99a
>>>> --- /dev/null
>>>> +++ b/tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c
>>>> @@ -0,0 +1,100 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +
>>>> +/*
>>>> + * Copyright (C) 2022 Huawei Technologies Duesseldorf GmbH
>>>> + *
>>>> + * Author: Roberto Sassu <roberto.sassu@huawei.com>
>>>> + */
>>>> +
>>>> +#include "vmlinux.h"
>>>> +#include <errno.h>
>>>> +#include <bpf/bpf_helpers.h>
>>>> +#include <bpf/bpf_tracing.h>
>>>> +
>>>> +#define MAX_DATA_SIZE (1024 * 1024)
>>>> +#define MAX_SIG_SIZE 1024
>>>> +
>>>> +typedef __u8 u8;
>>>> +typedef __u16 u16;
>>>> +typedef __u32 u32;
>>>> +typedef __u64 u64;
>>>
>>> I think you can avoid this and just use u32 and u64 directly.
>>
>> Thanks, yes.
>>
>>> +
>>>> +struct bpf_dynptr {
>>>> +       __u64 :64;
>>>> +       __u64 :64;
>>>> +} __attribute__((aligned(8)));
>>>> +
>>>
>>> I think you are doing this because including the uapi headers
>>> causes
>>> type conflicts.
>>> This does happen quite often. What do other folks think about doing
>>> something like
>>>
>>> #define DYNPTR(x) ((void *)x)
>>>
>>> It seems like this will be an issue anytime we use the helpers with
>>> vmlinux.h and users
>>> will always have to define this type in their tests.
>>
>> It seems it is sufficient to use struct bpf_dynptr somehow in the
>> kernel code. That causes the definition to be exported with BTF. Not
>> sure what would be the proper place to do that. When I tried, I
>> declared a unused variable.
> 
> Easier:
> 
> BTF_TYPE_EMIT(struct bpf_dynptr);
> 
> I added it in bpf_dynptr_from_mem(), right?

Yes, you can add it to a related function. The BTF_TYPE_EMIT
will be optimized out by the compiler.

> 
> Thanks
> 
> Roberto
> 