linux-kernel - Re: [PATCH sched_ext/for-6.15 v3 3/5] sched_ext: Add scx_kfunc_ids_ops_context_sensitive for unified filtering of context-sensitive SCX kfuncs

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:
 <AM6PR03MB50809C9EB32C9705DF6EA14299CC2@AM6PR03MB5080.eurprd03.prod.outlook.com>
Date: Fri, 28 Feb 2025 18:42:11 +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,
 arighi@...dia.com, changwoo@...lia.com, bpf@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH sched_ext/for-6.15 v3 3/5] sched_ext: Add
 scx_kfunc_ids_ops_context_sensitive for unified filtering of
 context-sensitive SCX kfuncs

On 2025/2/27 21:32, Tejun Heo wrote:
> Hello,
> 
> On Thu, Feb 27, 2025 at 09:23:20PM +0000, Juntong Deng wrote:
>>>> +	if (prog->type == BPF_PROG_TYPE_STRUCT_OPS &&
>>>> +	    prog->aux->st_ops != &bpf_sched_ext_ops)
>>>> +		return 0;
>>>
>>> Why can't other struct_ops progs call scx_kfunc_ids_unlocked kfuncs?
>>>
>>
>> Return 0 means allowed. So kfuncs in scx_kfunc_ids_unlocked can be
>> called by other struct_ops programs.
> 
> Hmm... would that mean a non-sched_ext bpf prog would be able to call e.g.
> scx_bpf_dsq_insert()?
> 

For other struct_ops programs, yes, in the current logic,
when prog->aux->st_ops != &bpf_sched_ext_ops, all calls are allowed.

This may seem a bit weird, but the reason I did it is that in other
struct_ops programs, the meaning of member_off changes, so the logic
that follows makes no sense at all.

Of course, we can change this, and ideally there would be some groupings
(kfunc id set) that declare which kfunc can be called by other
struct_ops programs and which cannot.

>>>> +	/* prog->type == BPF_PROG_TYPE_STRUCT_OPS && prog->aux->st_ops == &bpf_sched_ext_ops*/
>>>> +
>>>> +	moff = prog->aux->attach_st_ops_member_off;
>>>> +	flags = scx_ops_context_flags[SCX_MOFF_IDX(moff)];
>>>> +
>>>> +	if ((flags & SCX_OPS_KF_UNLOCKED) &&
>>>> +	    btf_id_set8_contains(&scx_kfunc_ids_unlocked, kfunc_id))
>>>> +		return 0;
>>>
>>> Wouldn't this disallow e.g. ops.dispatch() from calling scx_dsq_move()?
>>>
>>
>> No, because
>>
>>>> [SCX_OP_IDX(dispatch)] = SCX_OPS_KF_DISPATCH | SCX_OPS_KF_ENQUEUE,
>>
>> Therefore, kfuncs (scx_bpf_dsq_move_*) in scx_kfunc_ids_dispatch can be
>> called in the dispatch context.
> 
> I see, scx_dsq_move_*() are in both groups, so it should be fine. I'm not
> fully sure the groupings are the actually implemented filtering are in sync.
> They are intended to be but the grouping didn't really matter in the
> previous implementation. So, they need to be carefully audited.
> 

After you audit the current groupings of scx kfuncs, please tell me how
you would like to change the current groupings.

>>> Have you tested that the before and after behaviors match?
>>
>> I tested the programs in tools/testing/selftests/sched_ext and
>> tools/sched_ext and all worked fine.
>>
>> If there are other cases that are not covered, we may need to add new
>> test cases.
> 
> Right, the coverage there isn't perfect. Testing all conditions would be too
> much but it'd be nice to have a test case which at least confirms that all
> allowed cases verify successfully.
> 

Yes, we can add a simple test case for each operation that is not
SCX_OPS_KF_ANY.

> Thanks.
>