RFE: add test for validatetrans #76
Comments
|
Some additional context on the kernel oops: I discovered the kernel bug on a debian installation (linux-image-5.10.0-8-amd64, libsepol: 3.1, selinux policy version 32). But the exact policy doesn't seem to matter, you just need to be sure to actually trigger the evaluation of the validatetrans rule, then causing a BUG in |
|
That's ... interesting. I'm replying here before I've tried to reproduce this, I'm just looking at the kernel code and the only thing that immediately jumps out at me is the explicit Any chance you can convert the panic/oops func+offset into a file+line for us for your kernel? |
|
Does this help a bit more ? sysadmin@glados:~$ uname -a
Linux glados 5.10.0-8-amd64 #1 SMP Debian 5.10.46-1 (2021-06-24) x86_64 GNU/LinuxIf not then I might take a stab at converting the offset properly tomorrow. |
|
Hmm, so this happened pretty early during boot after the policy is loaded. Did you see this on v5.9 or earlier kernels? I'm beginning to wonder if this may be related to the policy RCU change ... |
|
Okay, so on v5.10.46 security/selinux/ss/services.c:381 is this: https://elixir.bootlin.com/linux/v5.10.46/source/security/selinux/ss/services.c#L381 ... which is a little confusing to read because the indenting appears to be messed up there (sigh). Unfortunately I need to turn off my computer for the evening right now, but if I'm reading this correctly it looks like the switch statement is all messed up - likely missed due to the bad indenting - and the |
|
The log I posted is from a system that was already running for some time (uptime of maybe an hour, but more than 20m). You can still see is the audit entry for my The BUG is then hit, probably when sudo tries to relabel the pty back to the user context. I'm not sure if this also occured prior to v5.9 as I don't think I've ever tried validatetrans. |
|
I couldn't resist a quick hack - that code is truly awful, and it looks to go back to at least 2005 (!) - this code is completely uncompiled, untested, etc. and even if it boots it basically short-circuits the constraint so it isn't a real fix, but it should at least not panic/BUG if you want to give it a shot. diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index d84c77f370dc..275eb676185d 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -356,29 +356,31 @@ static int constraint_expr_eval(struct policydb *policydb,
case CEXPR_L2H2:
l1 = &(tcontext->range.level[0]);
l2 = &(tcontext->range.level[1]);
- goto mls_ops;
mls_ops:
- switch (e->op) {
- case CEXPR_EQ:
- s[++sp] = mls_level_eq(l1, l2);
- continue;
- case CEXPR_NEQ:
- s[++sp] = !mls_level_eq(l1, l2);
- continue;
- case CEXPR_DOM:
- s[++sp] = mls_level_dom(l1, l2);
- continue;
- case CEXPR_DOMBY:
- s[++sp] = mls_level_dom(l2, l1);
- continue;
- case CEXPR_INCOMP:
- s[++sp] = mls_level_incomp(l2, l1);
- continue;
- default:
- BUG();
- return 0;
- }
- break;
+ switch (e->op) {
+ case CEXPR_EQ:
+ s[++sp] = mls_level_eq(l1, l2);
+ continue;
+ case CEXPR_NEQ:
+ s[++sp] = !mls_level_eq(l1, l2);
+ continue;
+ case CEXPR_DOM:
+ s[++sp] = mls_level_dom(l1, l2);
+ continue;
+ case CEXPR_DOMBY:
+ s[++sp] = mls_level_dom(l2, l1);
+ continue;
+ case CEXPR_INCOMP:
+ s[++sp] = mls_level_incomp(l2, l1);
+ continue;
+ default:
+ BUG();
+ return 0;
+ }
+ break;
+ case CEXPR_NAMES:
+ /* XXX - do something here */
+ return 1;
default:
BUG();
return 0;Okay, now I really need to go for the evening ... sorry :( |
|
I'm sorry, right now I don't have any infrastructure in place to compile a new kernel, nor do I really have the time for that 😞 |
|
The culprit is the expression in question is invalid: Probably the constraint shoud have been: since Checkpolicy(8) rejects such expressions and nowadays also secilc(8), since 3.3: SELinuxProject/selinux@e978e76. Nevertheless the kernel should not stop: either it should reject loading policies containing such invalid expressions, see https://patchwork.kernel.org/project/selinux/patch/20220222135037.30497-1-cgzones@googlemail.com/ for a userland equivalent, or/and use |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
There is actually a bug in this functionality as of at least SELinux 3.2/Linux 5.13 that causes a kernel oops when a validatetrans event is triggered.
The text was updated successfully, but these errors were encountered: