linux-kernel - Re: [RFC PATCH bpf-next 6/7] sched_ext: Make SCX use BPF capabilities

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:
 <AM6PR03MB5080A353CD7B20764053162C991B2@AM6PR03MB5080.eurprd03.prod.outlook.com>
Date: Fri, 17 Jan 2025 20:09:21 +0000
From: Juntong Deng <juntong.deng@...look.com>
To: Tejun Heo <tj@...nel.org>
Cc: ast@...nel.org, daniel@...earbox.net, john.fastabend@...il.com,
 andrii@...nel.org, martin.lau@...ux.dev, eddyz87@...il.com, song@...nel.org,
 yonghong.song@...ux.dev, kpsingh@...nel.org, sdf@...ichev.me,
 haoluo@...gle.com, jolsa@...nel.org, memxor@...il.com, void@...ifault.com,
 bpf@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH bpf-next 6/7] sched_ext: Make SCX use BPF capabilities

On 2025/1/17 16:58, Tejun Heo wrote:
> Hello,
> 
> On Thu, Jan 16, 2025 at 07:41:11PM +0000, Juntong Deng wrote:
> ...
>> +static int bpf_scx_bpf_capabilities_adjust(unsigned long *bpf_capabilities,
>> +					   u32 context_info, bool enter)
>> +{
>> +	if (enter) {
>> +		switch (context_info) {
>> +		case offsetof(struct sched_ext_ops, select_cpu):
>> +			ENABLE_BPF_CAPABILITY(bpf_capabilities, BPF_CAP_SCX_KF_SELECT_CPU);
>> +			ENABLE_BPF_CAPABILITY(bpf_capabilities, BPF_CAP_SCX_KF_ENQUEUE);
>> +			break;
> ...
>> +		}
>> +	} else {
>> +		switch (context_info) {
>> +		case offsetof(struct sched_ext_ops, select_cpu):
>> +			DISABLE_BPF_CAPABILITY(bpf_capabilities, BPF_CAP_SCX_KF_SELECT_CPU);
>> +			DISABLE_BPF_CAPABILITY(bpf_capabilities, BPF_CAP_SCX_KF_ENQUEUE);
>> +			break;
> ...
>> +	}
>> +	return 0;
>> +}
> 
>  From sched_ext's POV, this, or whatever works is fine but I have some basic
> comments:
> 
> - The caps are u32. If all kfuncs use this facility for access control, it's
>    plausible or even likely that 32 is not going to be enough. I suppose it
>    can be turned into u64 and then a proper bitmap later? Maybe good idea to
>    start out with a proper bitmap in the first place?
> 

Thanks for your reply.

I considered this, and 32 capabilities is definitely not enough.

So although in BTF_ID_FLAGS the capability is 32 bits, this is used as
an index and a bitmap is used in struct bpf_verifier_env.

We can have UINT_MAX quantity capability.

> - There are benefits to centralizing all the caps in a single place but it
>    can also be kinda cumbersome.
> 

Yes, I agree, maybe centralized declarations are cumbersome.

But the purpose of centralized declaration is to give each capability a
unique number for distinction.

I am not sure there is a non-centralized declarative way to do this.

Do you have any suggested approach?

> - Even with global defs, the cap adjustments are procedural, not
>    declarative. If it needs to be procedural anyway, I wonder whether the
>    global defs are necessary in the first place. What prevents implementing
>    it the other way around - pass in the calling context and provide helpers
>    and macros to respond yay or nay procedurally.
> 

You are right!

Thanks for pointing this out!

If it is procedural, then global definitions are really not necessary.

I will rethink this.

> Thanks.
>