On Ubuntu 20.04, when building with clang -Werror -Wextra-semi-stmt
(which is not the default build configuration), the compiler reports:
mcstransd.c:72:35: error: empty expression statement has no effect;
remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
log_debug("%s\n", "cleanup_exit");
^
Replace the empty log_debug substitution with a do { ... } while (0)
construction to silence this warning.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
On Ubuntu 20.04, when building with clang -Werror -Wextra-semi-stmt
(which is not the default build configuration), the compiler reports:
secon.c:686:3: error: empty expression statement has no effect;
remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
};
^
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
On Ubuntu 20.04, when building with clang -Werror -Wextra-semi-stmt
(which is not the default build configuration), the compiler reports:
checkpolicy.c:740:33: error: empty expression statement has no
effect; remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
FGETS(ans, sizeof(ans), stdin);
^
Introduce "do { } while (0)" blocks to silence such warnings.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
On Ubuntu 20.04, when building with clang -Werror -Wextra-semi-stmt
(which is not the default build configuration), the compiler reports:
genhomedircon.c:742:67: error: empty expression statement has no
effect; remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
const semanage_seuser_t **u2 = (const semanage_seuser_t **) arg2;;
^
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
On Ubuntu 20.04, when building with clang -Werror -Wextra-semi-stmt
(which is not the default build configuration), the compiler reports:
sha1.c:90:21: error: empty expression statement has no effect;
remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
R0(a,b,c,d,e, 0); R0(e,a,b,c,d, 1); R0(d,e,a,b,c, 2); R0(c,d,e,a,b, 3);
^
In file included from selinux_restorecon.c:39:
./label_file.h:458:15: error: empty expression statement has no
effect; remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
lineno);
^
Introduce "do { } while (0)" blocks to silence such warnings.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
On Ubuntu 20.04, when building with clang -Werror -Wextra-semi-stmt
(which is not the default build configuration), the compiler reports:
../cil/src/cil_binary.c:4293:22: error: empty expression statement
has no effect; remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
mix(k->target_class);
^
../cil/src/cil_binary.c:4294:21: error: empty expression statement
has no effect; remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
mix(k->target_type);
^
../cil/src/cil_binary.c:4295:21: error: empty expression statement
has no effect; remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
mix(k->source_type);
^
../cil/src/cil_binary.c:4296:19: error: empty expression statement
has no effect; remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
mix(k->specified);
^
Use a do { ... } while (0) construction to silence this warning.
Moreover the same warning appears when using two semicolons to end a
statement. Remove such occurrences, like what was already done in commit
811185648a ("libsepol: drop repeated semicolons").
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When __cil_verify_map_class() verifies a classpermission, it calls
__verify_map_perm_classperms() on each item. If the first item reports a
failure and the next one succeeds, the failure is overwritten in
map_args->rc. This is a bug which causes a NULL pointer dereference in
the CIL compiler when compiling the following policy:
(sid SID)
(sidorder (SID))
(class CLASS (PERM1))
(classorder (CLASS))
(classpermission CLSPERM)
(classpermissionset CLSPERM (CLASS (PERM1)))
(classmap files (CLAMAPxx x))
(classmapping files CLAMAPxx CLSPERM)
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=30286
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Fix the following build failure with gcc 4.8 which is raised since
version 3.2 and
156dd0de5c
getseuser.c:53:2: error: 'for' loop initial declarations are only allowed in C99 mode
for (int i = 0; i < n; i++)
^
Fixes:
- http://autobuild.buildroot.org/results/37eb0952a763256fbf6ef3c668f6c95fbdf2dd35
Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
Provide the option "-Q" or "--qualified-names" to indicate that the
policy is using qualified names.
Using qualified names means that declaration names can have "dots"
in them, but blocks, blockinherits, blockabstracts, and in-statements
are not allowed in the policy.
The libsepol function cil_set_qualified_names() is called with the
desired value for the CIL db's "qualified_names" field.
Signed-off-by: James Carter <jwcart2@gmail.com>
Provide the option "-Q" or "--qualified-names" to indicate that the
policy is using qualified names.
Using qualified names means that declaration names can have "dots"
in them, but blocks, blockinherits, blockabstracts, and in-statements
are not allowed in the policy.
The libsepol function cil_set_qualified_names() is called with the
desired value for the CIL db's "qualified_names" field.
Signed-off-by: James Carter <jwcart2@gmail.com>
Provide the option "-Q" or "--qualified-names" to indicate that the
policy is using qualified names.
Using qualified names means that declaration names can have "dots"
in them, but blocks, blockinherits, blockabstracts, and in-statements
are not allowed in the policy.
The libsepol function cil_set_qualified_names() is called with the
desired value for the CIL db's "qualified_names" field.
Signed-off-by: James Carter <jwcart2@gmail.com>
Qualified names have "dots" in them. They are generated when a CIL
policy is compiled and come from declarations in blocks. If a kernel
policy is decompiled into a CIL policy, the resulting policy could
have declarations that use qualified names. Compiling this policy would
result in an error because "dots" in declarations are not allowed.
Qualified names in a policy are normally used to refer to the name of
identifiers, blocks, macros, or optionals that are declared in a
different block (that is not a parent). Name resolution is based on
splitting a name based on the "dots", searching the parents up to the
global namespace for the first block using the first part of the name,
using the second part of the name to lookup the next block using the
first block's symbol tables, looking up the third block in the second's
symbol tables, and so on.
To allow the option of using qualified names in declarations:
1) Create a field in the struct cil_db called "qualified_names" which
is set to CIL_TRUE when qualified names are to be used. This field is
checked in cil_verify_name() and "dots" are allowed if qualified names
are being allowed.
2) Only allow the direct lookup of the whole name in the global symbol
table. This means that blocks, blockinherits, blockabstracts, and in-
statements cannot be allowed. Use the "qualified_names" field of the
cil_db to know when using one of these should result in an error.
3) Create the function cil_set_qualified_names() that is used to set
the "qualified_names" field. Export the function in libsepol.
Signed-off-by: James Carter <jwcart2@gmail.com>
When disabling optionals, the AST needs to be reset only if one
of the optional blocks being disabled contains a declaration.
Call the function cil_tree_subtree_has_decl() for each optional
block being disabled and only reset the AST if one of them has
a declaration in it.
Signed-off-by: James Carter <jwcart2@gmail.com>
Create the function cil_tree_subtree_has_decl() that returns CIL_TRUE
if the subtree has a declaration in it and CIL_FALSE otherwise.
Signed-off-by: James Carter <jwcart2@gmail.com>
The commit 74d00a8dec (libsepol/cil:
Detect degenerate inheritance and exit with an error) detects the
use of inheritance (mostly by the secilc-fuzzer and not in any real
policies) that results in the exponential growth of the policy through
the copying of blocks that takes place with inheritance in CIL.
Unfortunately, the check takes place during the pass when all the
blocks are being copied, so it is possible to consume all of a system's
memory before an error is produced.
The new check happens in two parts. First, a check is made while the
block inheritance is being linked to the block it will inherit. In
this check, all of the parent nodes of the inheritance rule up to the
root node are checked and if enough of these blocks are being inherited
(>= CIL_DEGENERATE_INHERITANCE_DEPTH), then a flag is set for a more
in-depth check after the pass. This in-depth check will determine the
number of potential inheritances that will occur when resolving the
all of the inheritance rules. If this value is greater than
CIL_DEGENERATE_INHERITANCE_GROWTH * the original number of inheritance
rules and greater than CIL_DEGENERATE_INHERITANCE_MINIMUM (which is
set to 0x1 << CIL_DEGENERATE_INHERITANCE_DEPTH), then degenerate
inheritance is determined to have occurred and an error result will
be returned.
Since the potential number of inheritances can quickly be an extremely
large number, the count of potential inheritances is aborted as soon
as the threshold for degenerate inheritance has been exceeded.
Normal policies should rarely, if ever, have the in-depth check occur.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
It is possible to create bad behaving policy that can consume all
of a system's memory (one way is through the use of inheritance).
Analyzing these policies shows that most of the memory usage is for
the block symtabs.
Most of the nineteen symtabs will most likely never be used, so give
these symtabs an initial size of 1. The others are given more
appropriate sizes.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When marking a type attribute as used in a neverallow (to help determine
whether or not it should be expanded), check if the attribute's expression
list is empty (no attributes are associated with it) before iterating
over the list.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When "glblub" was added as a default for the defaultrange rule, the
syntax array was updated because the "glblub" default does not need
to specify a range of "low", "high", or "low-high". Unfortunately,
additional checking was not added for the "source" and "target"
defaults to make sure they specified a range. This means that using
the "source" or "target" defaults without specifying the range will
result in a segfault.
When the "source" or "target" defaults are used, check that the rule
specifies a range as well.
This bug was found by the secilc-fuzzer.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Commit 61fbdce666 (ibsepol/cil: Check
for self-referential loops in sets) added checks for self-referential
loops in user, role, type, and category sets. Unfortunately, this
check ends up in an infinite loop if the set with the self-referential
loop is used in a different set that is checked before the bad set.
The problem with the old check is that only the initial datum is used
for the check. Instead, use a stack to track all of the set datums
that are currently involved as the check is made. A self-referential
loop occurs if a duplicate datum is found for any of the datums in the
stack.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
The commit d155b410d4 (libsepol/cil:
Check for duplicate blocks, optionals, and macros) fixed a bug
that allowed duplicate blocks, optionals, and macros with the same
name in the same namespace. For blocks and macros, a duplicate
is always a problem, but optional block names are only used for
in-statement resolution. If no in-statement refers to an optional
block, then it does not matter if more than one with same name
exists.
One easy way to generate multiple optional blocks with the same
name declaration is to call a macro with an optional block multiple
times in the same namespace.
As an example, here is a portion of CIL policy
(macro m1 ((type t))
(optional op1
(allow t self (CLASS (PERM)))
)
)
(type t1)
(call m1 (t1))
(type t2)
(call m1 (t2))
This will result in two optional blocks with the name op1.
There are three parts to allowing multiple optional blocks with
the same name declaration.
1) Track an optional block's enabled status in a different way.
One hinderance to allowing multiple optional blocks with the same
name declaration is that they cannot share the same datum. This is
because the datum is used to get the struct cil_optional which has
the enabled field and each block's enabled status is independent of
the others.
Remove the enabled field from struct cil_optional, so it only contains
the datum. Use a stack to track which optional blocks are being
disabled, so they can be deleted in the right order.
2) Allow multiple declarations of optional blocks.
Update cil_allow_multiple_decls() so that a flavor of CIL_OPTIONAL
will return CIL_TRUE. Also remove the check in cil_copy_optional().
3) Check if an in-statement refers to an optional with multiple
declarations and exit with an error if it does.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Make it more apparent that those data does not change and enforce it.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Clang issues:
module_to_cil.c:65:7: warning: no previous extern declaration for non-static variable 'out_file' [-Wmissing-variable-declarations]
FILE *out_file;
^
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
`hashtab_search()` does take `const_hashtab_key_t` as second parameter,
which is a typedef for `const char *`.
Drop the unnecessary and const-violating cast.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Initialize variables, as they are set after goto statements, which jump
to cleanup code using them.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
`const_hashtab_key_t` is a typedef of `const char *`, so these casts are
not needed.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
../cil/src/cil_binary.c:2230:24: warning: Value stored to 'cb_node' during its initialization is never read [deadcode.DeadStores]
struct cil_tree_node *cb_node = node->cl_head;
^~~~~~~ ~~~~~~~~~~~~~
Found by clang-analyzer
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
../cil/src/cil_write_ast.c:86:32: error: cast to smaller integer type 'enum cil_flavor' from 'void *' [-Werror,-Wvoid-pointer-to-enum-cast]
enum cil_flavor op_flavor = (enum cil_flavor)curr->data;
^~~~~~~~~~~~~~~~~~~~~~~~~~~
../cil/src/cil_write_ast.c:130:37: error: cast to smaller integer type 'enum cil_flavor' from 'void *' [-Werror,-Wvoid-pointer-to-enum-cast]
enum cil_flavor operand_flavor = (enum cil_flavor)curr->data;
^~~~~~~~~~~~~~~~~~~~~~~~~~~
Silence this warning by casting the pointer to an integer the cast to
enum cil_flavor.
See 32f8ed3d6b ("libsepol/cil: introduce intermediate cast to silence -Wvoid-pointer-to-enum-cast")
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
In case cats_ebitmap_len() returns 0, do not allocate but quit.
Found by clang-analyzer
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Make it more obvious which parameters are read-only and not being
modified and allow callers to pass const pointers.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Make it more obvious which parameters are read-only and not being
modified and allow callers to pass const pointers.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
conditional.c:391:4: warning: Value stored to 'i' is never read [deadcode.DeadStores]
i = 0;
^ ~
conditional.c:718:2: warning: Value stored to 'len' is never read [deadcode.DeadStores]
len = 0;
^ ~
conditional.c:772:2: warning: Value stored to 'len' is never read [deadcode.DeadStores]
len = 0;
^ ~
services.c:89:10: warning: Value stored to 'new_stack' during its initialization is never read [deadcode.DeadStores]
char **new_stack = stack;
^~~~~~~~~ ~~~~~
services.c:440:11: warning: Value stored to 'new_expr_list' during its initialization is never read [deadcode.DeadStores]
char **new_expr_list = expr_list;
^~~~~~~~~~~~~ ~~~~~~~~~
../cil/src/cil_binary.c:2230:24: warning: Value stored to 'cb_node' during its initialization is never read [deadcode.DeadStores]
struct cil_tree_node *cb_node = node->cl_head;
^~~~~~~ ~~~~~~~~~~~~~
Found by clang-analyzer
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Follow the project style of no declaration after statement.
Found by the gcc warning -Wdeclaration-after-statement
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Follow the project style of no declaration after statement.
Found by the gcc warning -Wdeclaration-after-statement
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Unsigned integer overflow is well-defined and not undefined behavior.
But it is still useful to enable undefined behavior sanitizer checks on
unsigned arithmetic to detect possible issues on counters or variables
with similar purpose.
Use a spaceship operator like comparison instead of subtraction.
Modern compilers will generate a single comparison instruction instead
of actually perform the subtraction.
policydb.c:826:17: runtime error: unsigned integer overflow: 24 - 1699 cannot be represented in type 'unsigned int'
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
The functions `role_set_get_role`, `sepol_validate_transition` and
`sepol_sidtab_remove` seem to be unused since the initial import.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Declare the functions as static or include the corresponding header
file.
assertion.c:294:5: error: no previous prototype for function 'report_assertion_failures' [-Werror,-Wmissing-prototypes]
int report_assertion_failures(sepol_handle_t *handle, policydb_t *p, avrule_t *avrule)
^
context.c:23:5: error: no previous prototype for function 'sepol_check_context' [-Werror,-Wmissing-prototypes]
int sepol_check_context(const char *context)
^
expand.c:3377:5: error: no previous prototype for function 'expand_cond_av_node' [-Werror,-Wmissing-prototypes]
int expand_cond_av_node(policydb_t * p,
^
policydb.c:638:6: error: no previous prototype for function 'role_trans_rule_destroy' [-Werror,-Wmissing-prototypes]
void role_trans_rule_destroy(role_trans_rule_t * x)
^
policydb.c:1169:5: error: no previous prototype for function 'policydb_index_decls' [-Werror,-Wmissing-prototypes]
int policydb_index_decls(sepol_handle_t * handle, policydb_t * p)
^
policydb.c:1429:6: error: no previous prototype for function 'ocontext_selinux_free' [-Werror,-Wmissing-prototypes]
void ocontext_selinux_free(ocontext_t **ocontexts)
^
policydb.c:1451:6: error: no previous prototype for function 'ocontext_xen_free' [-Werror,-Wmissing-prototypes]
void ocontext_xen_free(ocontext_t **ocontexts)
^
policydb.c:1750:5: error: no previous prototype for function 'type_set_or' [-Werror,-Wmissing-prototypes]
int type_set_or(type_set_t * dst, type_set_t * a, type_set_t * b)
^
policydb.c:2524:5: error: no previous prototype for function 'role_trans_read' [-Werror,-Wmissing-prototypes]
int role_trans_read(policydb_t *p, struct policy_file *fp)
^
policydb.c:2567:5: error: no previous prototype for function 'role_allow_read' [-Werror,-Wmissing-prototypes]
int role_allow_read(role_allow_t ** r, struct policy_file *fp)
^
policydb.c:2842:5: error: no previous prototype for function 'filename_trans_read' [-Werror,-Wmissing-prototypes]
int filename_trans_read(policydb_t *p, struct policy_file *fp)
^
services.c:1027:5: error: no previous prototype for function 'sepol_validate_transition' [-Werror,-Wmissing-prototypes]
int sepol_validate_transition(sepol_security_id_t oldsid,
^
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Christian Göttsche <cgzones@googlemail.com> submitted a similar patch
to quote paths when generating CIL policy from a binary policy.
Since genfscon and devicetreecon rules have paths which are allowed
to contain spaces, always quote the path when writing out these rules.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
CIL has rules that allow names to be assigned to certain objects
like MLS category sets, MLS levels, MLS ranges, IP addresses, and
class permission sets. These objects can also be named as parameters
for a macro. A call may pass in a name for one of these objects, but
it also may pass in one of the actual objects. These objects are
referred as anonymous arguments.
Add CIL policy that can be used to test whether or not anonymous
arguments are being handled properly in macros. Also test the
equivalent named arguments to help determine if the problem is with
that argument type or just with an anonymous argument of that type.
The anonymouse arguments that are tested are categoryset, level,
levelrange, ipaddr, and classpermission.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
It is possible for anonymous category sets to be in a category
expression if the expression has a macro parameter in it.
Unfortunately, anonymous category sets are not looked for when
resolving category expressions and a segfault will occur during
later processing if there was one.
As an example, consider the following portion of a policy.
(macro m1 ((categoryset cs))
(userlevel USER (s0 (cs)))
)
(call m1 ((c0 c1)))
This policy will cause a segault, because the categoryset datum
for the parameter cs is not seen as a categoryset and is treated
as a plain category.
When resolving an expression, check whether or not the datum that
is found is actually an anonymous category set associated with a
macro parameter. If it is, then resolve the category set if it
has not already been resolved and treat its categories as a sub
expression.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
A named IP address (using an ipaddr rule) could be passed as an
argument, but trying to pass an actual IP address caused an error.
As an exmample, consider the following portion of a policy.
(macro m4 ((ipaddr ip)(ipaddr nm))
(nodecon ip nm (USER ROLE TYPE ((s0) (s0))))
)
(ipaddr nm1 255.255.255.0)
(ipaddr ip1 1.2.3.4)
(call m4 (ip1 nm1)) ; This works
(call m4 (1.2.3.4 255.255.255.0)) ; This doesn't
Allow actual IP addresses to be passed as a call argument. Now the
second call works as well.
Signed-off-by: James Carter <jwcart2@gmail.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
The passing parameter "arg" of parse_module_store will be freed after
calling. A copy of parameter should be used instead of itself.
Signed-off-by: HuaxinLu <luhuaxin1@foxmail.com>
Acked-by: James Carter <jwcart2@gmail.com>
Using mount flag `nosuid` also affects SELinux domain transitions but
this has not been documented well.
Signed-off-by: Topi Miettinen <toiwoton@gmail.com>
When generating CIL policy from kernel or module policy quote paths,
which are allowed to contain spaces, in the statements `genfscon` and
`devicetreecon`.
Reported by LuK1337 while building policy for Android via IRC.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Improve formatting of section DESCRIPTION by adding list points.
Mention errno is set on failure.
Mention the returned context might be NULL if SELinux is not enabled.
Align setcon/_raw parameter by adding const.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
