On Tue, Jan 31, 2023 at 12:04 AM Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > > On Mon, Jan 30, 2023 at 06:47:54PM +0000, Ori Glassman wrote: > > > > > security@kernel alias is not the place to ask bpf related questions. > > I apologise if it was confusing, I wasn't asking a question - rather raising a security concern of mine. > > That is a security concern with a design of your service. > sec@kern is about kernel bugs: > Documentation/admin-guide/security-bugs.rst > > > > Yaniv, from your team, already brought it up here: > > > https://lore.kernel.org/bpf/CAADnVQ++LzKt9Q-GtGXknVBqyMqY=vLJ3tR3NNGG3P66gvVCFQ@xxxxxxxxxxxxxx/ > > > > > You cannot assume that different bpf progs attached to various > > > events like tracepoints and lsm hooks won't overlap. > > > It's a bug in your program. Nothing else. > > How can one use bpf_task_storage_get() without the risk of getting corrupted? Say there's a module that consists of 1 simple program, a single LSM hook on bprm_creds_for_exec, that uses a local task storage pinned map. > > An attacker at some point in the future loads his tracepoint program, and maliciously corrupts the local storage *while* the LSM hook is executing, not after or before. What's the bug in the program that consists of the single LSM hook? So, your MAC policy allows unprivileged users (i.e. the attacker) to tamper with security blobs (i.e. task_local_storage). This is a bad idea, your policy should prevent the helper being accessible to other programs. Rather, it should ideally prevent loading of untrusted BPF programs completely. - KP > > The local storage map is also accessible via bpf syscall map_update_elem command. > If user can get an FD to that map they can update it. > It's your job to design the service such that bpf maps are not laying around for everyone to poke into. > But if user has root access it can do anything. > > > > These two programs access some task local storage. > > I'm talking specifically when the programs are executed by the same task and thus accessing the same local storage. > > > This code racy regardless of preempt_disable vs migrate_disable. > > > bpf_task_storage_get() of the same task can run on different cpus. > > Not at the same time though, right? I'm not concerned about the cases where the map is used in multiple programs - I'm concerned about the cases where it's used locally in a single program, but gets corrupted in a timely manner from the outside by an attacker. > > At the same time. > Two cpus can observe the same task local storage. > > > > Whether trace_sched_process_free and security_bprm_creds_for_exec > > > can happen on different cpus is kernel implementation detail. > > > > > There looks to be another bug in the above: > > > doing bpf_get_current_task_btf from raw_tracepoint/sched_process_free > > > will return task_struct of the worker thread. > > > I don't think it's the one you want. > > That's not what I observed - this is the output of bpf_trace_printk where the execution of the LSM hook got interrupted mid-execution: > > ---- > > chrony-onofflin-12460 [000] d.s.1 2258.804195: bpf_trace_printk: EXECUTION HIJACK(b=2257261931167) # this is from tp/sched_process_free > > chrony-onofflin-12460 [000] d...1 2258.804234: bpf_trace_printk: a=2257261896666. c=2257261971220 # from the lsm hook > > It's "working" by accident.
