fix(driver): fix potential deadlock

[therealbobo]

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind feature

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area API-version

/area build

/area CI

/area driver-kmod

/area driver-bpf

/area driver-modern-bpf

/area libscap-engine-bpf

/area libscap-engine-gvisor

/area libscap-engine-kmod

/area libscap-engine-modern-bpf

/area libscap-engine-nodriver

/area libscap-engine-noop

/area libscap-engine-source-plugin

/area libscap-engine-savefile

/area libscap

/area libpman

/area libsinsp

/area tests

/area proposals

Does this PR require a change in the driver versions?

/version driver-API-version-major

/version driver-API-version-minor

/version driver-API-version-patch

/version driver-SCHEMA-version-major

/version driver-SCHEMA-version-minor

/version driver-SCHEMA-version-patch

What this PR does / why we need it:

Kernels < 3.1.0 doesn't support the exe_writable flags due to the MAY_NOT_BLOCK not being available. This limitation is related to the fact that the f_sched_prog_exec function is in a RCU critical section: this means that this function (and its callee) MUST NOT call functions that can yield the processor (e.g. inode_permission that deep down in its call stack calls a down_read()). This is addressed after the Kernel 3.1.0 where the MAY_OT_BLOCK flag is introduced and avoids the processor to being yield.

Given all of that does make sense to avoid filling the exe_writable flag in Kernels <3.1.0.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

fix(driver): `exe_writable` is not being filled with the correct value with kernels <3.1.0

[github-actions]

Please double check driver/API_VERSION file. See versioning.

/hold

[poiana]

LGTM label has been added.

Git tree hash: 0153e4deaeaa7ba976b741c7eebf6afb5633c7bb

[FedeDP]

/hold
Going to run kernel tests against this one ;)

[FedeDP]

https://github.com/falcosecurity/libs/actions/runs/7573675028
But hey I forgot that libs master broke kernel tests :/ no luck, need to wait until that is resolved first.

[jcpittman144]

A couple of remarks:

• The function f_proc_startupdate(), in this same file, has the same logic, and needs the same fix. In fact, it is the code in f_proc_startupdate() that is causing the problem at the customer site.
• I also checked to see if the BPF code has the same logic. But in that code, there is just a comment that says "Basic file permission check -- may not work in all cases, cannot inspect in eBPF". I guess it is possible to have the kernel module do the same thing as the eBPF logic? Because we are trying to move customers to eBPF, and if this safer approach is good enough in eBPF, maybe it is good enough for the kernel module too?

[therealbobo]

Ops, I missed the other function. Sorry! Just fixed.

[LucaGuerra]

Thanks @jcpittman144 ! We are actually doing this in two different places, makes sense. Re. the eBPF code: I believe the approach we use in the kmod for this kind of things should work better because we are in a position to use locks, while with eBPF we are more limited (read only, no loops...) and we sometimes need to implement workarounds.

@therealbobo : were you able to test if the flag behaves properly after your latest patch? Thanks!

[therealbobo]

Hey @LucaGuerra! Sorry for the delay 😅 I just tested the new patch and it works correctly! 😄

[FedeDP]

Kernel tests are finally fixed; going to run them against this branch ;)

[FedeDP]

ARM64:

KERNEL	CMAKE-CONFIGURE	KMOD BUILD	KMOD SCAP-OPEN	BPF-PROBE BUILD	BPF-PROBE SCAP-OPEN	MODERN-BPF SCAP-OPEN
amazonlinux2-5.4	🟢	🟢	🟢	🟢	🟢	🟡
amazonlinux2022-5.15	🟢	🟢	🟢	🟢	🟢	🟢
fedora-6.2	🟢	🟢	🟢	🟢	🟢	🟢
oraclelinux-4.14	🟢	🟢	🟢	🟡	🟡	🟡
oraclelinux-5.15	🟢	🟢	🟢	🟢	🟢	🟢
ubuntu-6.3	🟢	🟢	🟢	🟢	🟢	🟢

AMD64:

KERNEL	CMAKE-CONFIGURE	KMOD BUILD	KMOD SCAP-OPEN	BPF-PROBE BUILD	BPF-PROBE SCAP-OPEN	MODERN-BPF SCAP-OPEN
amazonlinux2-4.19	🟢	🟢	🟢	🟢	🟢	🟡
amazonlinux2-5.10	🟢	🟢	🟢	🟢	🟢	🟢
amazonlinux2-5.15	🟢	🟢	🟢	🟢	🟢	🟢
amazonlinux2-5.4	🟢	🟢	🟢	🟢	🟢	🟡
amazonlinux2022-5.15	🟢	🟢	🟢	🟢	🟢	🟢
amazonlinux2023-6.1	🟢	🟢	🟢	🟢	🟢	🟢
archlinux-6.0	🟢	🟢	🟢	🟢	🟢	🟢
centos-3.10	🟢	🟢	🟢	🟡	🟡	🟡
centos-4.18	🟢	❌	❌	🟢	🟢	🟢
centos-5.14	🟢	🟢	🟢	🟢	🟢	🟢
fedora-5.17	🟢	❌	❌	🟢	🟢	🟢
fedora-5.8	🟢	🟢	🟢	🟢	🟢	🟢
fedora-6.2	🟢	🟢	🟢	🟢	🟢	🟢
oraclelinux-3.10	🟢	🟢	🟢	🟡	🟡	🟡
oraclelinux-4.14	🟢	🟢	🟢	🟢	🟢	🟡
oraclelinux-5.15	🟢	🟢	🟢	🟢	🟢	🟢
oraclelinux-5.4	🟢	🟢	🟢	🟢	🟢	🟡
ubuntu-4.15	🟢	🟢	🟢	🟢	🟢	🟡
ubuntu-6.3	🟢	🟢	🟢	🟢	🟢	🟢

No changes from master.

[FedeDP]

/approve

[poiana]

LGTM label has been added.

Git tree hash: f419b49af1a9d2113a82df26eff1ded87eb061a0

[poiana]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, LucaGuerra, therealbobo

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:

• driver/OWNERS [FedeDP,LucaGuerra]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[FedeDP]

@therealbobo can you bump the patch API_VERSION?

[Andreagit97]

@therealbobo can you bump the patch API_VERSION?

why do we need to bump the API_VERSION? we are not touching the userspace-driver APIs 🤔

[FedeDP]

I think we decided to always bump driver versioning even for this kind of changes.

[Andreagit97]

uhm IIRC we never did it :/

it seems like we are in this case:

patch increases either when API_VERSION’s patch or SCHEMA_VERSION’s patch number has been increased or when any other code changes have been introduced (for example, the support for a new kernel)

https://github.com/falcosecurity/libs/blob/master/proposals/20220203-versioning-schema-amendment.md#proposed-solution

More in detail:

or when any other code changes have been introduced (for example, the support for a new kernel)

IMO these kinds of changes don't touch the API/DRIVER versions but if we need to tag a new driver version they cause a patch increase in the overall driver version

[FedeDP]

oh it always confuses me, thanks!
/unhold