fix(events): restore dependency in hooked_syscall

[NDStrahilevitz]

1. Explain what the PR does

6298835:

fix(events): restore dependency in hooked_syscall
    
The event dependency for do_init_module in the hooked_syscall event was
removed in commit 43a3eac. As a result, installing a kernel module did
not immediately trigger a syscall table integrity check.

Fix #3783

2. Explain how to test it

See issue.

[rafaeldtinoco]

IIRC we discussed this during the review of that code change.

[NDStrahilevitz]

IIRC we discussed this during the review of that code change.

I just checked #3544 and didn't find such a discussion. Is it possible the discussion was internal?
Anyway, there are still references to that code in the processing of do_init_module, and there's no comment explaining the removal in the code. I've also talked with @AlonZivony before opening the PR, he said he saw no issue in bringing it back, though it is @OriGlassman who developed, so his input would be good here.

[rafaeldtinoco]

IIRC we discussed this during the review of that code change.

I just checked #3544 and didn't find such a discussion. Is it possible the discussion was internal? Anyway, there are still references to that code in the processing of do_init_module, and there's no comment explaining the removal in the code. I've also talked with @AlonZivony before opening the PR, he said he saw no issue in bringing it back, though it is @OriGlassman who developed, so his input would be good here.

I just tried to bring him to the discussion because of a "timing" logic that currently exists and was discussed between him and me (#3544 (comment)).

Before, as soon as you loaded the kernel module, if there was a hooked syscall you would get right away (after the module was loaded). Now, there is a time window between checks (for hooked syscalls).

NOTE: I didnt check if its directly related to the lack of the dependency, nor went to the code to check, nor said I agree or disagree to your change (fyio).

[NDStrahilevitz]

Before, as soon as you loaded the kernel module, if there was a hooked syscall you would get right away (after the module was loaded). Now, there is a time window between checks (for hooked syscalls).

Yes, and that is because the dependency was removed. FWIW from this discussion it could mean it was intentional on his part. Regardless, for the product, this doesn't work so well for whatever reason. I don't want to add another configuration flag just for this event (we need event parameters/settings whatever for this) to decide between the two methods, so I propose just having them live side by side for now.
Unrelated but the timing window is random (between 10 to 300 seconds), and I'm not sure that is documented anywhere, and it's not configurable.

NOTE: I didnt check if its directly related to the lack of the dependency, nor went to the code to check, nor said I agree or disagree to your change (fyio).

No harm done, don't worry.

[rafaeldtinoco]

Unrelated but the timing window is random (between 10 to 300 seconds), and I'm not sure that is documented anywhere, and it's not configurable.

The change was intentional and the range randomness was also intentional. They wanted to avoid overloading the logic with loads/unloads (or too many loads) or something like that iirc. Whatever research and product team agrees seems fine, there is no right or wrong, just useful or not. Its good that you're looking into it.

[OriGlassman]

There's support for both periodic triggering and per module load triggering (in do_init_module userspace code:

)
So it's indeed a bug.
do_init_module dependency should be added as it is the focal point for triggering other logic such as mem dump and seq ops.

A note about using do_init_module: The reason we use do_init_module, obviously, is that it's where the init code of the module is ran and we wanna check if the init code did something suspicious.

Using kprobe on do_init_module to do this, might trigger the logic (such as hooked_syscall event) before the init code of the module actually runs.
Since time passes between kernelspace until do_init_module userspace code in tracee runs, most of the times the init functions of the module finishes.
In cases that the init code execution time is long, we might run before the "interesting" part of the init code of the module actually runs, so this should probably be a kretprobe.

I'll leave a side note that using kretprobe has a downside unfortunately, since an attacker might hang the do_init_module (by using sleep etc), and the kretprobe will never fire. This can be addressed by checking for hanging do_init_module, or maybe by using some other hook (if exist) other than do_init_module.

[NDStrahilevitz]

@OriGlassman Thanks for the detailed response!

@rafaeldtinoco FYI looks like this can go ahead.

[rafaeldtinoco]

LGTM after explanations.