78 lines
2.1 KiB
Diff
78 lines
2.1 KiB
Diff
|
From 64387cb37379fc8f135eeecd2bd9fdf3c591c763 Mon Sep 17 00:00:00 2001
|
||
|
|
From: Nicolas Iooss <nicolas.iooss@m4x.org>
|
||
|
|
Date: Sat, 3 Oct 2020 15:34:19 +0200
|
||
|
|
Subject: [PATCH] libsepol: drop confusing BUG_ON macro
|
||
|
|
|
||
|
|
Contrary to Linux kernel, BUG_ON() does not halt the execution, in
|
||
|
|
libsepol/src/services.c. Instead it displays an error message and
|
||
|
|
continues the execution.
|
||
|
|
|
||
|
|
This means that this code does not prevent an out-of-bound write from
|
||
|
|
happening:
|
||
|
|
|
||
|
|
case CEXPR_AND:
|
||
|
|
BUG_ON(sp < 1);
|
||
|
|
sp--;
|
||
|
|
s[sp] &= s[sp + 1];
|
||
|
|
|
||
|
|
Use if(...){BUG();rc=-EINVAL;goto out;} constructions instead, to make
|
||
|
|
sure that the array access is always in-bound.
|
||
|
|
|
||
|
|
This issue has been found using clang's static analyzer:
|
||
|
|
https://558-118970575-gh.circle-artifacts.com/0/output-scan-build/2020-10-02-065849-6375-1/report-50a861.html#EndPath
|
||
|
|
|
||
|
|
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
|
||
|
|
---
|
||
|
|
libsepol/src/services.c | 19 +++++++++++++++----
|
||
|
|
1 file changed, 15 insertions(+), 4 deletions(-)
|
||
|
|
|
||
|
|
diff --git a/libsepol/src/services.c b/libsepol/src/services.c
|
||
|
|
index 90da1f4efef3..beb0711f6680 100644
|
||
|
|
--- a/libsepol/src/services.c
|
||
|
|
+++ b/libsepol/src/services.c
|
||
|
|
@@ -67,7 +67,6 @@
|
||
|
|
#include "flask.h"
|
||
|
|
|
||
|
|
#define BUG() do { ERR(NULL, "Badness at %s:%d", __FILE__, __LINE__); } while (0)
|
||
|
|
-#define BUG_ON(x) do { if (x) ERR(NULL, "Badness at %s:%d", __FILE__, __LINE__); } while (0)
|
||
|
|
|
||
|
|
static int selinux_enforcing = 1;
|
||
|
|
|
||
|
|
@@ -469,18 +468,30 @@ static int constraint_expr_eval_reason(context_struct_t *scontext,
|
||
|
|
/* Now process each expression of the constraint */
|
||
|
|
switch (e->expr_type) {
|
||
|
|
case CEXPR_NOT:
|
||
|
|
- BUG_ON(sp < 0);
|
||
|
|
+ if (sp < 0) {
|
||
|
|
+ BUG();
|
||
|
|
+ rc = -EINVAL;
|
||
|
|
+ goto out;
|
||
|
|
+ }
|
||
|
|
s[sp] = !s[sp];
|
||
|
|
cat_expr_buf(expr_list[expr_counter], "not");
|
||
|
|
break;
|
||
|
|
case CEXPR_AND:
|
||
|
|
- BUG_ON(sp < 1);
|
||
|
|
+ if (sp < 1) {
|
||
|
|
+ BUG();
|
||
|
|
+ rc = -EINVAL;
|
||
|
|
+ goto out;
|
||
|
|
+ }
|
||
|
|
sp--;
|
||
|
|
s[sp] &= s[sp + 1];
|
||
|
|
cat_expr_buf(expr_list[expr_counter], "and");
|
||
|
|
break;
|
||
|
|
case CEXPR_OR:
|
||
|
|
- BUG_ON(sp < 1);
|
||
|
|
+ if (sp < 1) {
|
||
|
|
+ BUG();
|
||
|
|
+ rc = -EINVAL;
|
||
|
|
+ goto out;
|
||
|
|
+ }
|
||
|
|
sp--;
|
||
|
|
s[sp] |= s[sp + 1];
|
||
|
|
cat_expr_buf(expr_list[expr_counter], "or");
|
||
|
|
--
|
||
|
|
2.29.0.rc2
|
||
|
|
|