linux-kernel - Re: [RFC PATCH bpf-next 0/3] bpf: freeze a task cgroup from bpf

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: <CAADnVQLSDOfKccynu2jt-7=8nJqoLtoNkRchvHo1NCUEYOQJ7Q@mail.gmail.com>
Date: Thu, 28 Mar 2024 14:28:51 -0700
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Tejun Heo <tj@...nel.org>
Cc: Djalal Harouni <tixxdz@...il.com>, Zefan Li <lizefan.x@...edance.com>, 
	Johannes Weiner <hannes@...xchg.org>, Alexei Starovoitov <ast@...nel.org>, 
	Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, 
	Martin KaFai Lau <martin.lau@...ux.dev>, Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>, 
	Yonghong Song <yonghong.song@...ux.dev>, John Fastabend <john.fastabend@...il.com>, 
	KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>, 
	Jiri Olsa <jolsa@...nel.org>, Mykola Lysenko <mykolal@...com>, Shuah Khan <shuah@...nel.org>, 
	LKML <linux-kernel@...r.kernel.org>, 
	"open list:CONTROL GROUP (CGROUP)" <cgroups@...r.kernel.org>, bpf <bpf@...r.kernel.org>, 
	"open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@...r.kernel.org>
Subject: Re: [RFC PATCH bpf-next 0/3] bpf: freeze a task cgroup from bpf

On Thu, Mar 28, 2024 at 2:01 PM Tejun Heo <tj@...nel.org> wrote:
>
> Hello,
>
> On Thu, Mar 28, 2024 at 01:45:56PM -0700, Alexei Starovoitov wrote:
> > On Thu, Mar 28, 2024 at 1:02 PM Tejun Heo <tj@...nel.org> wrote:
> > >
> > > There's also cgroup.kill which would be useful for similar use cases. We can
> > > add interface for both but idk. Let's say we have something like the
> > > following (pardon the bad naming):
> > >
> > >   bpf_cgroup_knob_write(struct cgroup *cgrp, char *filename, char *buf)
> > >
> > > Would that work? I'm not necessarily in love with the idea or against adding
> > > separate helpers but the duplication still bothers me a bit.
> >
> > I liked it.
> > So filename will be one of cgroup_base_files[].name ?
> > We probably don't want psi or cgroup1_base_files in there.
>
> Would it matter?

Few weak reasons:
 cgroup_psi_files have show/write/poll/release which
  doesn't map to this bpf_cgroup_knob_write/read ?
 cgroup1_base_files probably needs to a separate kfunc
  bpf_cgroup1_...

> If the user has root perm, they can do whatever with the
> files anyway, so I'm not sure why we'd restrict any specific knob. Maybe we
> wanna make sure @filename doesn't include '/'? Or is it that you don't want
> to go through the usual file name look up?

yeah. why do a file lookup? The names are there in the array.
cgroup pointer gives that "relative path" and knob name is the last
part of such "path". Easy to search in that array(s).

> > From the verifier pov 2nd arg can be "char *knob__str" and
> > the verifier will make sure it's a constant NULL terminated string,
> > so at runtime it will be easier to search cgroup_base_files array.
> > And 'buf' can be: void *mem, int mem__sz with kfunc doing
> > run-time validation that there a null there.
>
> That all sound good.
>
> Thanks.
>
> --
> tejun