fix(scripts): driver loader insmod #2388
Conversation
scripts/debian/postinst.in
Outdated
| echo "[POST-INSTALL] Call 'falco-driver-loader bpf':" | ||
| falco-driver-loader bpf | ||
| echo "[POST-INSTALL] Call 'falco-driver-loader --compile bpf':" | ||
| falco-driver-loader --compile bpf |
There was a problem hiding this comment.
@leogr do we need to compile-only the bpf probe?
Perhaps we can download it too, instead.
There was a problem hiding this comment.
Or even better, we might want to add the falco-driver-loader --compile X in falco-kmod and falco-bpf.service, as PreStart?
This way, we could enforce that a driver is always properly loaded.
There was a problem hiding this comment.
Since in a local system, one is supposed to have all required dependencies installed (i.e., kernel headers), I'd not use the download approach. It could create more issues than benefits (for example, think in an air-gapped env).
For the kmod use case, assuming that dkms will automatically rebuild the module when the local kernel is upgraded, I think the post-install approach is enough. It should work in all situations.
For the ebpf case, it likely won't. Immagine the following situation:
- the current kernel version is X
- the user installs Falco, and the ebpf gets built for X
- the user upgrades the kernel to Y
- the system is rebooted
- Falco won't work anymore because the probe was built for X
I don't have a definitive answer yet, but the best option is likely to leave the kmod at the post-install stage and only move the bpf at the PreStart stage. 🤔 (Although I don't like this kinda asymmetry).
|
/milestone 0.34.0 |
|
/hold |
scripts/debian/postinst.in
Outdated
| echo "[POST-INSTALL] Call 'falco-driver-loader bpf':" | ||
| falco-driver-loader bpf | ||
| echo "[POST-INSTALL] Call 'falco-driver-loader --compile bpf':" | ||
| falco-driver-loader --compile bpf |
There was a problem hiding this comment.
Since in a local system, one is supposed to have all required dependencies installed (i.e., kernel headers), I'd not use the download approach. It could create more issues than benefits (for example, think in an air-gapped env).
For the kmod use case, assuming that dkms will automatically rebuild the module when the local kernel is upgraded, I think the post-install approach is enough. It should work in all situations.
For the ebpf case, it likely won't. Immagine the following situation:
- the current kernel version is X
- the user installs Falco, and the ebpf gets built for X
- the user upgrades the kernel to Y
- the system is rebooted
- Falco won't work anymore because the probe was built for X
I don't have a definitive answer yet, but the best option is likely to leave the kmod at the post-install stage and only move the bpf at the PreStart stage. 🤔 (Although I don't like this kinda asymmetry).
In this way, dkms will gracefully handle kernels updates. Signed-off-by: Federico Di Pierro <[email protected]>
…der. Signed-off-by: Federico Di Pierro <[email protected]>
FedeDP
force-pushed
the
fix/driver_loader_isnmod
branch
from
e882d44 to
fac86c6
Compare
| @@ -8,6 +8,7 @@ Wants=falcoctl-artifact-follow.service | |||
| Type=simple | |||
| User=root | |||
| Environment=FALCO_BPF_PROBE= | |||
| ExecStartPre=/usr/bin/falco-driver-loader bpf | |||
There was a problem hiding this comment.
We chose to enable download too for ebpf probe, because it has a clang dep that might not be satisfied and we don't want to enforce it.
Moreover, given that ebpf probe is not managed by the system (dkms) it's ok to manage it ourselves.
Finally, note that if the correct ebpf probe is already in place, the falco-driver-loader script won't do anything: https://github.com/falcosecurity/falco/blob/master/scripts/falco-driver-loader#L585
|
/unhold |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: FedeDP, jasondellaluce, leogr The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind bug
Any specific area of the project related to this PR?
What this PR does / why we need it:
Reverts 3087362; we always need to use
insmodto avoid weird situations (eg: multiple drivers being installed at the same time, and modprobe choosing the oldest one).Moreover, for package/host installations, enforce the usage of dkms, with
--compileflag.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: