CONFIG_GRKERNSEC_HIDESYM infoleak warnings #8999
Comments
|
The plot thickens. When i build with CONFIG_ZFS=m, the ZFS module is not being built or packaged somehow. After a rebuild as module, can't find the module, no sysfs entries, etc. Seems it might not be building the entire thing somehow. Rebuilding kernel with logs to see what the deuce is going on. |
|
So the failure was somehow silently passing and allowing the package/kernel to build, which is an issue in itself, but with build logging enabled it actually freaked out and died @ So in the LEFT argument being cast as a uint64_t is not valid. I need to read up on ZCP and its internals, figure out if this is something in my built system or in ZFS. Anyone else seeing this? |
|
@kpande, i imagine gentoo is GCC 9.1 @ this point? |
|
Thank you. Maybe an older toolchain will work... |
|
oh it doesnt, zfs failed to build, but as i mentioned the fact that the final image was built without linking in ZFS and friends is a whole other issue :) |
|
So i've got it building, and running zloop without blowing up, yet anyway. |
|
ehm. wtf. linux 5.2.0-rc7, gcc 8.3.0-6, zfs-0.8.0-ga3c1a8a03 from my fork... works perfectly... |
|
I think this is an issue with my build-stack, so closing out as "not upsteam's problem." |
|
I just ran into the exact same build error while building ZFS built-in (not as a module) as described here: #8999 (comment) Gentoo, GCC 9.1.0-r1 (default compiler on ~amd64 / Gentoo testing), kernel 4.14.132 The workaround mentioned in #8999 (comment) allows for the compiliation to proceed. |
|
In fact, this does appear to be our problem. I suspect your build environment has The issue is the cast in the VERIFY3U which isn't correct since the diff --git a/module/zfs/zcp_get.c b/module/zfs/zcp_get.c
index ed98f0d..0a5f0b8 100644
--- a/module/zfs/zcp_get.c
+++ b/module/zfs/zcp_get.c
@@ -423,13 +423,11 @@ get_special_prop(lua_State *state, dsl_dataset_t *ds, cons
case ZFS_PROP_RECEIVE_RESUME_TOKEN: {
char *token = get_receive_resume_stats_impl(ds);
- VERIFY3U(strlcpy(strval, token, ZAP_MAXVALUELEN),
- <, ZAP_MAXVALUELEN);
+ (void) strlcpy(strval, token, ZAP_MAXVALUELEN);
if (strcmp(strval, "") == 0) {
char *childval = get_child_receive_stats(ds);
- VERIFY3U(strlcpy(strval, childval, ZAP_MAXVALUELEN),
- <, ZAP_MAXVALUELEN);
+ (void) strlcpy(strval, childval, ZAP_MAXVALUELEN);
if (strcmp(strval, "") == 0)
error = ENOENT;
|
|
Yes! My build environment does have _FORTIFY_SOURCE set to 2 and applying your patch resolves the problem for me. |
|
@gcs-github thanks! Could you also check if your kernel is built with |
The cast of the size_t returned by strlcpy() to a uint64_t by the VERIFY3U can result in a build failure when CONFIG_FORTIFY_SOURCE is set. This is due to the additional hardening. Since the token will always fit in strval the VERIFY3U has been removed. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#8999
It is! |
|
@behlendorf my kernel config has FORTIFY_SOURCE=y, zfs in-tree has |
|
In light of @c0d3z3r0 's comment and since I hadn't realized that @sempervictus also had 20MB of additional patching (I read too quickly and wrongly assumed a vanilla setup): some extra patching also exists on my end:
Since you identified the problem as an erroneous cast, I suppose it was a good idea to fix it anyway, but still: I'm sorry. I'll make sure to try and reproduce any issue on vanilla kernel sources before reporting or confirming a problem in the future, unless the issue or the discussion is itself about the support of a specific kernel modfication. |
|
@gcs-github the kernel gcc patch is ok, I have that, too I am pretty sure this is caused / revealed by the grsecurity patch, which both of you have |
|
No problem, regardless I think we're going to want to remove the VERIFY3U. Alternately, if we really want to keep the sanity check I suspect that changing it to a VERIFY3S would also resolve the build issue. Though, someone who can reproduce the issue would need to perform a test build. |
|
Turns out, VERIFY3S causes the same |
|
@gcs-github I think that's it pending review feedback on the PR. Thanks! |
|
@gcs-github: same here, though the way that ZFS patch is generated is a bit quirky (configure is patched too, but that can only apply cleanly in the same build conditions as the ones under which autogen was run). So far, just to get it building/running, the hack above seems to do the trick. |
|
Seems there might be more of these things lurking around, i'm still seeing this at runtime: same thing for metaslab_alloc as metaslab_sync, apparently we're leaking raw pointers in the process of __dprintf-ing things. |
|
I think a887d65 might be the culprit here since it switches from %p to %px. I sed -i'd all instances of %px with %p in the in-kernel tree, running a build/test cycle presently. Hopefully will know more by tonight. |
|
Nope, starts up with and then spams this on ever metaslab_alloc apparently: So, lets see if can address that. |
|
@behlendorf the pointer printing thing should probably take into account whether upstream's kptr_restrict is enabled, and determine based on that whether to %p or %px. The VERIFY3P thing i'm a bit confused about (ASSERT3P being VERIFY3P with debug enabled or nothing without it), since i already converted it from %px to %p in the last patch, but thats the only place in metaslab_alloc i could find which would end up printing a formatted pointer. |
|
No, metaslab_alloc and metaslab_sync still spamming. |
According to the documentation,
Looks at the code, I don't see where this is coming from immediately. You could try ruling out all of the |
|
Thank you, %pK is the correct version of that. Strangely the |
|
@sempervictus I'm reproducing your #8999 (comment) on ZFS 0.8.1 + grsec but it seems triggered by grsecurity's infoleak detection mechanism, not by ZFS debugging routines. edit: Unless you meant that the debug code is itself what's causing the leaks. |
|
@sempervictus a note re: #8999 (comment) ; one way you can work around the inflexibility (which is also what I did to start testing with zfs 0.8.1) is to simply remove the patch lines related to the configure script (and in the case of zfs 0.8.1, the ones related to the toplevel Makefile.in as well). The relevant changes in the configure script seem to be generated through |
|
@gcs-github I'm not familiar with the grsecurity's infoleak detection mechanism, do you know what changes would need to be made to ZFS code to make it happy? Could you refer me to any documentation? Patches welcome! |
The cast of the size_t returned by strlcpy() to a uint64_t by the VERIFY3U can result in a build failure when CONFIG_FORTIFY_SOURCE is set. This is due to the additional hardening. Since the token is expected to always fit in strval the VERIFY3U has been removed. If somehow it doesn't, it will still be safely truncated. Reviewed-by: Tony Hutter <hutter2@llnl.gov> Reviewed-by: Don Brady <don.brady@delphix.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue #8999 Closes #9020
|
@behlendorf Looking inside the code, I tracked down the dmesg infoleak warning to this code block: https://github.com/minipli/linux-unofficial_grsec/blob/linux-4.9.x-unofficial_grsec/lib/vsprintf.c#L1753-L1757 (this comes from minipli's 4.9 port of the last publicly-available grsecurity patch) GRKERNSEC_HIDESYM is a grsecurity feature aiming to hide kernel symbols from unprivileged users. Its full Kconfig help text is available @ https://github.com/minipli/linux-unofficial_grsec/blob/linux-4.9.x-unofficial_grsec/grsecurity/Kconfig#L201-L224 I suppose the infoleak is therefore a symbol leak somehow. I'm unaware of any developer-oriented documentation for grsecurity. I've only come across user-facing documentation so far. For the sake of completeness, here's one of the dmesg traces I reproduced: |
behlendorf
changed the title
|
Thanks for the link. It looks like switching to |
The cast of the size_t returned by strlcpy() to a uint64_t by the VERIFY3U can result in a build failure when CONFIG_FORTIFY_SOURCE is set. This is due to the additional hardening. Since the token is expected to always fit in strval the VERIFY3U has been removed. If somehow it doesn't, it will still be safely truncated. Reviewed-by: Tony Hutter <hutter2@llnl.gov> Reviewed-by: Don Brady <don.brady@delphix.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#8999 Closes openzfs#9020
The cast of the size_t returned by strlcpy() to a uint64_t by the VERIFY3U can result in a build failure when CONFIG_FORTIFY_SOURCE is set. This is due to the additional hardening. Since the token is expected to always fit in strval the VERIFY3U has been removed. If somehow it doesn't, it will still be safely truncated. Reviewed-by: Tony Hutter <hutter2@llnl.gov> Reviewed-by: Don Brady <don.brady@delphix.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#8999 Closes openzfs#9020
|
I went into my local copy of the ZFS module code and replaced most of the %p and %px occurences with %pK, for testing purposes. This successfully suppressed the dmesg traces I mentioned above. What I'm not clear on is how those traces got triggered in the first place. %p and %px occurences in ZFS seem connected to debug messaging and problem reporting, so I would still have expected to see something related to ZFS in the dmesg output at runtime, but I haven't so far. |
|
So the %p/px replacements are one thing, but it seems there are some really weird hits to dprintf which aren't in the C code: which i dont see anywhere in dmu_buf_rele_array. Maybe some values interpreted as pointers are making it into vsnprintf somehow... i've expanded the check above to print the fmt passed into it to try and determine what's going on there, but for the meantime, on my actual work system, i've just commented out the guts of __dprintf since it cant be disabled when built-in to the kernel binary (not just in-tree as a module, but directly built-in). For any other grsec users reading this, the reason to use built-in as opposed to module-based is that RAP and KERNEXEC require the BTS instruction for module instrumentation when SMAP isnt available, VMs dont have the relevant subsystem much less instruction, so this permits use of those plugins without SMAP in a VM. |
behlendorf
added
the
Type: Building
The cast of the size_t returned by strlcpy() to a uint64_t by the VERIFY3U can result in a build failure when CONFIG_FORTIFY_SOURCE is set. This is due to the additional hardening. Since the token is expected to always fit in strval the VERIFY3U has been removed. If somehow it doesn't, it will still be safely truncated. Reviewed-by: Tony Hutter <hutter2@llnl.gov> Reviewed-by: Don Brady <don.brady@delphix.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#8999 Closes openzfs#9020
The cast of the size_t returned by strlcpy() to a uint64_t by the VERIFY3U can result in a build failure when CONFIG_FORTIFY_SOURCE is set. This is due to the additional hardening. Since the token is expected to always fit in strval the VERIFY3U has been removed. If somehow it doesn't, it will still be safely truncated. Reviewed-by: Tony Hutter <hutter2@llnl.gov> Reviewed-by: Don Brady <don.brady@delphix.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#8999 Closes openzfs#9020
The cast of the size_t returned by strlcpy() to a uint64_t by the VERIFY3U can result in a build failure when CONFIG_FORTIFY_SOURCE is set. This is due to the additional hardening. Since the token is expected to always fit in strval the VERIFY3U has been removed. If somehow it doesn't, it will still be safely truncated. Reviewed-by: Tony Hutter <hutter2@llnl.gov> Reviewed-by: Don Brady <don.brady@delphix.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#8999 Closes openzfs#9020
The cast of the size_t returned by strlcpy() to a uint64_t by the VERIFY3U can result in a build failure when CONFIG_FORTIFY_SOURCE is set. This is due to the additional hardening. Since the token is expected to always fit in strval the VERIFY3U has been removed. If somehow it doesn't, it will still be safely truncated. Reviewed-by: Tony Hutter <hutter2@llnl.gov> Reviewed-by: Don Brady <don.brady@delphix.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#8999 Closes openzfs#9020
The cast of the size_t returned by strlcpy() to a uint64_t by the VERIFY3U can result in a build failure when CONFIG_FORTIFY_SOURCE is set. This is due to the additional hardening. Since the token is expected to always fit in strval the VERIFY3U has been removed. If somehow it doesn't, it will still be safely truncated. Reviewed-by: Tony Hutter <hutter2@llnl.gov> Reviewed-by: Don Brady <don.brady@delphix.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#8999 Closes openzfs#9020
The cast of the size_t returned by strlcpy() to a uint64_t by the VERIFY3U can result in a build failure when CONFIG_FORTIFY_SOURCE is set. This is due to the additional hardening. Since the token is expected to always fit in strval the VERIFY3U has been removed. If somehow it doesn't, it will still be safely truncated. Reviewed-by: Tony Hutter <hutter2@llnl.gov> Reviewed-by: Don Brady <don.brady@delphix.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#8999 Closes openzfs#9020
The cast of the size_t returned by strlcpy() to a uint64_t by the VERIFY3U can result in a build failure when CONFIG_FORTIFY_SOURCE is set. This is due to the additional hardening. Since the token is expected to always fit in strval the VERIFY3U has been removed. If somehow it doesn't, it will still be safely truncated. Reviewed-by: Tony Hutter <hutter2@llnl.gov> Reviewed-by: Don Brady <don.brady@delphix.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#8999 Closes openzfs#9020
The cast of the size_t returned by strlcpy() to a uint64_t by the VERIFY3U can result in a build failure when CONFIG_FORTIFY_SOURCE is set. This is due to the additional hardening. Since the token is expected to always fit in strval the VERIFY3U has been removed. If somehow it doesn't, it will still be safely truncated. Reviewed-by: Tony Hutter <hutter2@llnl.gov> Reviewed-by: Don Brady <don.brady@delphix.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue #8999 Closes #9020
|
This issue has been automatically marked as "stale" because it has not had any activity for a while. It will be closed in 90 days if no further activity occurs. Thank you for your contributions. |
and others
System information
Describe the problem you're observing
When copying the ZFS sources into the kernel tree and configuring the build to directly integrate ZFS into the kernel binary (not as a module), both the zfs and zpool commands return:
This is new behavior (trying to finalize the jump from 0.7), and ztest does work somehow (running in the background presently testing away under zloop)
Describe how to reproduce the problem
Create a builtin ZFS structure within a kernel tree, configure the kernel with CONFIG_ZFS=y, build, install, build tools for the same version, run ztest -VVV, run zfs list while ztest is running.
Include any warning/errors/backtraces from the system logs
Nothing in dmesg.
Strace says:
Which is odd because the sysfs directory was still there with the ZFS 0.7.13 builtin.
The text was updated successfully, but these errors were encountered: