[SMAGENT-1783] Don't check the signal info if it's not valid #1493
Conversation
This is the eBPF half of pull request #1460
driver/bpf/fillers.h
Outdated
| sig = ctx->sig; | ||
| if (sig == SIGKILL) { | ||
|
|
||
| if (info == SEND_SIG_NOINFO || info == SEND_SIG_PRIV || info == SEND_SIG_FORCED) { |
There was a problem hiding this comment.
I missed SEND_SIG_FORCED in my change. I think it should be in both the regular+bpf drivers, but both places need to add a version check. It's been removed in later kernel versions according to https://lore.kernel.org/patchwork/patch/981479/.
Have you tried building with a recent kernel? If we have compilation problems and add version checks, let's also update the regular driver code.
There was a problem hiding this comment.
Ooh, good catch, thanks! Definitely doesn't build on a 5.2 kernel!
It's just a macro, so I think a simple #ifdef might be more straightforward than a version check. I'll update both the kmod and the bpf driver accordingly!
driver/bpf/fillers.h
Outdated
| #ifdef SEND_SIG_FORCED | ||
| #define SIGINFO_NOT_A_POINTER(_info) ((_info) <= SEND_SIG_FORCED) | ||
| #else | ||
| #define SIGINFO_NOT_A_POINTER(_info) ((struct kernel_siginfo*)(_info) <= SEND_SIG_PRIV) |
There was a problem hiding this comment.
Why cast for this one and not the other #define? We should use a function and get a little bit of type-checking safety.
driver/bpf/fillers.h
Outdated
| @@ -3518,6 +3518,12 @@ FILLER(sys_pagefault_e, false) | |||
| return res; | |||
| } | |||
|
|
|||
| #ifdef SEND_SIG_FORCED | |||
| #define SIGINFO_NOT_A_POINTER(_info) ((_info) <= SEND_SIG_FORCED) | |||
There was a problem hiding this comment.
I don't think <= is defined behavior for pointers, and also we're relying on the implementation of SEND_SIG_*. We should explicitly check for each SEND_SIG_* type.
There was a problem hiding this comment.
In the patch that you helpfully sent me, look at is_sig_special. Looks like that's what the kernel itself uses (would be helpful if they just exported that dang function and saved us some trouble...).
If you'd rather me explicitly check each type though I can do so.
There was a problem hiding this comment.
Let's check explicitly. I would love to use the kernel implementation, and I doubt it will ever change, but there's nothing to guarantee that.
This is the eBPF half of pull request #1460