Title
#general
a

Alexander

03/15/2021, 6:17 PM
Thank you
Stefano Bonicatti

Stefano Bonicatti

03/15/2021, 6:34 PM
So heads up, updating the toolchain might be slightly more involved. The CI (and the common build) always uses a previously built and released version of the toolchain. Doing this release requires several steps. Currently the repository is more up to date vs the version of the toolchain that's released, and while I wanted to do a new release with the toolchain using LLVM 11, I realized that we have to correct the usage of the kernel headers. They were set to version 4.9 back then due to some BPF dependencies but this is partially incorrect because we support CentOS 6. What we should do toolchain wise is actually to compile against CentOS 6 kernel version of headers, then osquery has to define the newer kernels defines and at runtime detect if some feature is available or not.
6:38 PM
But I also need to actually explore this... because there might effectively be issues with the process of preparing the toolchain itself when setting such old headers. Nonetheless, I would say that the PR currently should not require an update of the toolchain, but provide the define if missing; it should also work correctly on kernel versions that do not possess the latest features.
a

Alexander

03/15/2021, 6:47 PM
Got it, we can just define missing constants in our source, but that requires some "ugly" decisions, like should we define only constants that are missing in 4.7, or all used constants. I though updating headers in toolchain would be better than trying to make questionable constants in osquery itself, that would become redundant in future
Stefano Bonicatti

Stefano Bonicatti

03/15/2021, 6:53 PM
I see what you mean; my interest is mainly trying not to block your PR and at the same time try to adhere to what I was saying earlier. I haven't checked specifically, but lets say we're not updating the toolchain right now, which constants have to be defined for the code to compile? Updating the toolchain really needs some tests to see how it works with older distros, that's something we really want to have.
a

Alexander

03/15/2021, 7:04 PM
I should recheck, as that patch was done a year before (we used it internally for a year). Most probably the missing constants are: SECCOMP_RET_KILL_PROCESS and SECCOMP_RET_LOG, both introduced in 4.14 https://github.com/torvalds/linux/commit/4d3b0b05aae9ee9ce0970dc4cc0fb3fad5e85945
Stefano Bonicatti

Stefano Bonicatti

03/15/2021, 7:06 PM
I see, then that might be enough for that issue. Thank you for understanding!
a

Alexander

03/15/2021, 7:06 PM
SECCOMP_RET_KILL_THREAD also need to be checked, accouding to
man seccomp
it replaced SECCOMP_RET_KILL in 4.14
Stefano Bonicatti

Stefano Bonicatti

03/15/2021, 7:09 PM
Maybe that's a rename only though.
7:12 PM
though in the diff you linked it seems they are both still present; seems what they have done is to add a
SECCOMP_RET_KILL_PROCESS
and so instead of referring to the more generic
SECCOMP_RET_KILL
they use
SECCOMP_RET_KILL_THREAD
to differentiate, but they have the same value.
a

Alexander

03/15/2021, 7:14 PM
Yes, but that``SECCOMP_RET_KILL_THREAD` not in 4.13 source, so there must be some other 4.14 patch that introduced new constant.
Stefano Bonicatti

Stefano Bonicatti

03/15/2021, 7:16 PM
oh I see, I was assuming they were both present and only manual changed. Nonetheless it seems to me that
SECCOMP_RET_KILL
retained its original value, I'm looking at 4.9 code
7:16 PM
so the value didn't change anyway
7:17 PM
But yeah the define might have to be added to the osquery source code. Sorry I was thinking you were referring to some value change
a

Alexander

03/15/2021, 7:19 PM
Ok. @Anton Zhabolenko, I think you can read our thread.
7:26 PM
BTW, @Stefano Bonicatti can you advise where should we put those ifndef-s in osquery tree? We can just put it here - close to
#include <linux/seccomp.h>
or make some fancy wrapper to seccomp.h, but i don't see appropriate place for wrappers. And should it be in-tree, or it better to be some logic around kernel version in CMakeFile?
Stefano Bonicatti

Stefano Bonicatti

03/15/2021, 7:31 PM
That's a good question. For simplicity I would just put them inline for now, under the
linux/seccomp.h
header. That is the only usage now and I suspect for a while too. We'll handle moving them somewhere else if needed. But definitely if you can, put them under
ifndef
so that we can play nice with other kind of builds.
a

Anton Zhabolenko

03/16/2021, 10:15 AM
We did it and checks passed. Thank you!