fix(scripts/falco-driver-loader): lsmod usage

[dnwe]

What type of PR is this?

/kind bug

Any specific area of the project related to this PR?

/area engine

What this PR does / why we need it:

Attempting to start falco on a host that had a similarly named module
(e.g., "falcon") would cause the falco-driver-loader to loop attempting
to rmmod falco when falco was not loaded.

falco-driver-loader will now inspect only the first column of lsmod
output and require the whole search string to match

Which issue(s) this PR fixes:

Fixes #1468

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

fix(scripts/falco-driver-loader): lsmod usage

[poiana]

Welcome @dnwe! It looks like this is your first PR to falcosecurity/falco 🎉

[dnwe]

/assign @fntlnz

[leogr]

Thank you for your contribution!

We definitively need this fix.
/milestone 0.27.0

Just a question, see below.

[leodido]

I have some concerns on the portability of -q flag.

... unlike GNU grep, 7th Edition Unix grep did not conform to POSIX, because it lacked -q and its -s option behaved like GNU grep's -q option. USG-style grep also lacked -q but its -s option behaved like GNU grep. Portable shell scripts should avoid both -q and -s and should redirect standard and error output to /dev/null instead.

From the manual pages.

Also, we could think of using /proc/modules rather than lsmod. WDYT?

[dnwe]

@leodido I did originally suggest reading from /proc/modules in my issue #1468 as it has the advantage of non-root and file reading rather than a process spawn, but there seemed to be some concerns and it isn’t necessary to fix the bug so I reverted to lsmod

[dnwe]

RE: portability of grep -q — whilst this was probably an issue historically, I checked AIX, Busybox, FreeBSD, HP-UX, OpenBSD and Solaris variants of grep, and they all support the expected -q behaviour

[leogr]

@leodido I did originally suggest reading from /proc/modules in my issue #1468 as it has the advantage of non-root and file reading rather than a process spawn, but there seemed to be some concerns and it isn’t necessary to fix the bug so I reverted to lsmod

Regarding my comment about /proc/modules: although my concern about its availability in a container context, I don't have a strong opinion on that, and I'm not against its usage. Feel free to go with /proc/modules if it helps, and we are sure there are no portability issues.

Thank you again.

[leodido]

RE: portability of grep -q — whilst this was probably an issue historically, I checked AIX, Busybox, FreeBSD, HP-UX, OpenBSD and Solaris variants of grep, and they all support the expected -q behaviour

I was referring to what man pages say. Not checked personally, by hand.

So, if the -q flag has been included in (non GNU) greps and it's not a portability issue anymore as you saying, the last remaining topic to get this merged is understand if the /proc/modules approach has limitations or not.

BTW great job, this is really needed 👏

[dnwe]

RE: container runtimes, I checked the only two I have to hand (docker and containerd). In both cases if I start a barebones container with --cap-drop all and --user nobody I confirmed that I was able to read from /proc/modules file without issue so I think it would probably be fine.

However, there's no great motivation to change it when the falco-driver-loader script is already running as root anyway

[poiana]

LGTM label has been added.

Git tree hash: 3a5aa8848cedd78d5794bcb6353e4a6cf34d90e5

[poiana]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: leodido, 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:

• OWNERS [leodido,leogr]

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