-
Notifications
You must be signed in to change notification settings - Fork 377
cgroup, systemd: validate ebpf is loaded #1865
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's GuideThe PR adds a mechanism to query attached eBPF programs on a cgroup and integrates a verification step into the systemd cgroup entry path, causing an early failure if a requested eBPF device filter isn’t actually installed. Sequence diagram for systemd cgroup entry with eBPF device filter verificationsequenceDiagram
participant Caller
participant libcrun_cgroup_enter_systemd
participant verify_ebpf_device_filter_installed
participant libcrun_ebpf_query_cgroup_progs
participant cgroup
Caller->>libcrun_cgroup_enter_systemd: Enter systemd cgroup
libcrun_cgroup_enter_systemd->>verify_ebpf_device_filter_installed: If bpf_dev_set, verify filter
verify_ebpf_device_filter_installed->>libcrun_ebpf_query_cgroup_progs: Query attached eBPF programs
libcrun_ebpf_query_cgroup_progs->>cgroup: Open cgroup path and read programs
cgroup-->>libcrun_ebpf_query_cgroup_progs: Return program list
libcrun_ebpf_query_cgroup_progs-->>verify_ebpf_device_filter_installed: Return program count
verify_ebpf_device_filter_installed-->>libcrun_cgroup_enter_systemd: Return success or error
libcrun_cgroup_enter_systemd-->>Caller: Return success or error
Class diagram for new and updated eBPF-related functionsclassDiagram
class libcrun_ebpf_query_cgroup_progs {
+int libcrun_ebpf_query_cgroup_progs(const char *cgroup_path, uint32_t **progs_out, size_t *n_progs_out, libcrun_error_t *err)
}
class verify_ebpf_device_filter_installed {
+int verify_ebpf_device_filter_installed(const char *cgroup_path, libcrun_error_t *err)
}
class libcrun_ebpf_read_program {
+int libcrun_ebpf_read_program(struct bpf_program **program_ret, const char *path, libcrun_error_t *err)
}
class libcrun_ebpf_cmp_programs {
+bool libcrun_ebpf_cmp_programs(struct bpf_program *program1, struct bpf_program *program2)
}
libcrun_ebpf_query_cgroup_progs <|-- verify_ebpf_device_filter_installed
Class diagram for updated systemd cgroup entry logicclassDiagram
class libcrun_cgroup_enter_systemd {
+int libcrun_cgroup_enter_systemd(struct libcrun_cgroup_args *args, ...)
+calls verify_ebpf_device_filter_installed if bpf_dev_set
}
class verify_ebpf_device_filter_installed {
+int verify_ebpf_device_filter_installed(const char *cgroup_path, libcrun_error_t *err)
}
libcrun_cgroup_enter_systemd --> verify_ebpf_device_filter_installed
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
50e58d1
to
fefe6e1
Compare
Ephemeral COPR build failed. @containers/packit-build please check. |
1 similar comment
Ephemeral COPR build failed. @containers/packit-build please check. |
systemd ignores errors with BPFProgram so make sure the ebpf program is installed when requested. Alternative to containers#1854 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
fefe6e1
to
a111995
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Extract the logic for stripping the systemd scope suffix from the cgroup path into a small helper function to improve readability and reduce duplication.
- In libcrun_ebpf_query_cgroup_progs the HAVE_EBPF disabled path returns an error instead of an empty list; consider returning zero programs (no error) or documenting that callers must handle this specific unsupported‐feature error.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Extract the logic for stripping the systemd scope suffix from the cgroup path into a small helper function to improve readability and reduce duplication.
- In libcrun_ebpf_query_cgroup_progs the HAVE_EBPF disabled path returns an error instead of an empty list; consider returning zero programs (no error) or documenting that callers must handle this specific unsupported‐feature error.
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
TMT tests failed. @containers/packit-build please check. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this looks much better.
LGTM
given the reason in systemd/systemd#38732, this is probably the best we can do. @kolyshkin what do you think? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
systemd ignores errors with BPFProgram so make sure the ebpf program is installed when requested.
Alternative to #1854
Summary by Sourcery
Validate that systemd has installed the requested eBPF device filter on the cgroup by querying attached programs
Enhancements: