[PATCH] libsepol/cil: Destroy cil_tree_node stacks when finished resolving AST
CIL uses separate cil_tree_node stacks for optionals and blocks to
check for statements not allowed in optionals or blocks and to know
which optional to disable when necessary. But these stacks were not
being destroyed when exiting cil_resolve_ast(). This is not a problem
normally because the stacks will be empty, but this is not the case
when exiting with an error.
Destroy both tree node stacks when exiting to ensure that they are
empty.
Signed-off-by: James Carter
---
libsepol/cil/src/cil_resolve_ast.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/libsepol/cil/src/cil_resolve_ast.c
b/libsepol/cil/src/cil_resolve_ast.c
index 7fe4a74..6628dc4 100644
--- a/libsepol/cil/src/cil_resolve_ast.c
+++ b/libsepol/cil/src/cil_resolve_ast.c
@@ -3778,6 +3778,16 @@ exit:
return rc;
}
+static void cil_destroy_tree_node_stack(struct cil_tree_node *curr)
+{
+ struct cil_tree_node *next;
+ while (curr != NULL) {
+ next = curr->cl_head;
+ free(curr);
+ curr = next;
+ }
+}
+
int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current)
{
int rc = SEPOL_ERR;
@@ -3904,16 +3914,12 @@ int cil_resolve_ast(struct cil_db *db, struct
cil_tree_node *current)
/* reset the arguments */
changed = 0;
while (extra_args.optstack != NULL) {
- struct cil_tree_node *curr = extra_args.optstack;
- struct cil_tree_node *next = curr->cl_head;
- free(curr);
- extra_args.optstack = next;
+ cil_destroy_tree_node_stack(extra_args.optstack);
+ extra_args.optstack = NULL;
}
while (extra_args.blockstack!= NULL) {
- struct cil_tree_node *curr = extra_args.blockstack;
- struct cil_tree_node *next = curr->cl_head;
- free(curr);
- extra_args.blockstack= next;
+ cil_destroy_tree_node_stack(extra_args.blockstack);
+ extra_args.blockstack = NULL;
}
}
@@ -3924,6 +3930,8 @@ int cil_resolve_ast(struct cil_db *db, struct
cil_tree_node *current)
rc = SEPOL_OK;
exit:
+ cil_destroy_tree_node_stack(extra_args.optstack);
+ cil_destroy_tree_node_stack(extra_args.blockstack);
__cil_ordered_lists_destroy(&extra_args.sidorder_lists);
__cil_ordered_lists_destroy(&extra_args.classorder_lists);
__cil_ordered_lists_destroy(&extra_args.catorder_lists);
--
2.7.4
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH 1/1] libsepol/cil: fix type confusion in cil_copy_ast
On 02/05/2017 09:14 AM, Nicolas Iooss wrote:
When running secilc on the following CIL file, the program tries to free
the data associated with type X using cil_destroy_typeattribute():
(macro sys_obj_type ((user ARG1)) (typeattribute X))
(block B
(type X)
(call sys_obj_type (Y))
)
By adding some printf statements to cil_typeattribute_init(),
cil_type_init() and cil_destroy_typeattribute(), the error message I get
when using gcc's address sanitizer is:
$ secilc -o /dev/null -f /dev/null test.cil -vv
creating TYPE 0x6040dfd0
Parsing 2017-02-02_crashing_nulptrderef_cil.cil
Building AST from Parse Tree
creating TYPEATTR 0x6060e420
creating TYPE 0x6040df50
Destroying Parse Tree
Resolving AST
Failed to resolve call statement at 2017-02-02_crashing_nulptrderef_cil.cil:5
Problem at 2017-02-02_crashing_nulptrderef_cil.cil:5
Pass 8 of resolution failed
Failed to resolve ast
Failed to compile cildb: -2
Destroying TYPEATTR 0x6060e420, types (nil) name X
Destroying TYPEATTR 0x6040df50, types 0xbebebebe name X
ASAN:DEADLYSIGNAL
=
==30684==ERROR: AddressSanitizer: SEGV on unknown address
0x (pc 0x7fc0539d114a bp 0x7ffc1fbcb300 sp
0x7ffc1fbcb2f0 T0)
#0 0x7fc0539d1149 in ebitmap_destroy
/usr/src/selinux/libsepol/src/ebitmap.c:356
#1 0x7fc053b96201 in cil_destroy_typeattribute
../cil/src/cil_build_ast.c:2370
#2 0x7fc053b42ea4 in cil_destroy_data ../cil/src/cil.c:616
#3 0x7fc053c595bf in cil_tree_node_destroy ../cil/src/cil_tree.c:235
#4 0x7fc053c59819 in cil_tree_children_destroy ../cil/src/cil_tree.c:201
#5 0x7fc053c59958 in cil_tree_subtree_destroy ../cil/src/cil_tree.c:172
#6 0x7fc053c59a27 in cil_tree_destroy ../cil/src/cil_tree.c:165
#7 0x7fc053b44fd7 in cil_db_destroy ../cil/src/cil.c:299
#8 0x4026a1 in main /usr/src/selinux/secilc/secilc.c:335
#9 0x7fc0535e5290 in __libc_start_main (/usr/lib/libc.so.6+0x20290)
#10 0x403af9 in _start (/usr/src/selinux/DESTDIR/usr/bin/secilc+0x403af9)
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /usr/src/selinux/libsepol/src/ebitmap.c:356 in
ebitmap_destroy
==30684==ABORTING
When copying the AST tree in cil_resolve_call1(),
__cil_copy_node_helper() calls cil_copy_typeattribute() to grab type X
in the symbol table of block B, and creates a node with the data of X
but with CIL_TYPEATTRIBUTE flavor.
This example is a "type confusion" bug between cil_type and
cil_typeattribute structures. It can be generalized to any couple of
structures sharing the same symbol table (an easy way of finding other
couples is by reading the code of cil_flavor_to_symtab_index()).
Fix this issue in a "generic" way in __cil_copy_node_helper(), by
verifying that the flavor of the found data is the same as expected and
triggering an error when it is not.
Signed-off-by: Nicolas Iooss
Applied with one addition. I added the line "new->flavor = FLAVOR(data);" to
make sure the data flavor matches. I don't think that that is needed because
cil_destroy_data() will not be called on the new node (since a previous node
exists), but I would rather be safe and have the flavor match.
Thanks,
Jim
---
libsepol/cil/src/cil_copy_ast.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/libsepol/cil/src/cil_copy_ast.c b/libsepol/cil/src/cil_copy_ast.c
index 5debd0d5359c..17a8c991c66d 100644
--- a/libsepol/cil/src/cil_copy_ast.c
+++ b/libsepol/cil/src/cil_copy_ast.c
@@ -1987,6 +1987,13 @@ int __cil_copy_node_helper(struct cil_tree_node *orig,
__attribute__((unused)) u
new->data = data;
if (orig->flavor >= CIL_MIN_DECLARATIVE) {
+ /* Check the flavor of data if was found in the
destination symtab */
+ if (DATUM(data)->nodes->head && FLAVOR(data) !=
orig->flavor) {
+ cil_tree_log(orig, CIL_ERR, "Incompatible flavor when
trying to copy %s", DATUM(data)->name);
+ cil_tree_log(NODE(data), CIL_ERR, "Note: conflicting
declaration");
+ rc = SEPOL_ERR;
+ goto exit;
+ }
rc = cil_symtab_insert(symtab, ((struct
cil_symtab_datum*)orig->data)->name, ((struct cil_symtab_datum*)data), new);
namespace = new;
--
James Carter
National Security Agency
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH 1/1] Introduce Travis-CI tests
On 02/05/2017 06:40 AM, Nicolas Iooss wrote:
Add a configuration file for https://travis-ci.org/. This continuous
integration platform can build the project for several configurations on
Linux, using different compilers, linkers, Python versions and Ruby
versions. An example of build results is available on
https://travis-ci.org/fishilico/selinux/builds/185912863
Even if the SELinux userland libraries and tools project does not enable
Travis-CI integration, the .travis.yml file may be helpful for
contributors who wish to run tests in several configurations.
Current limitations:
- It does not run an OS X build. Travis-CI provides free OS X
environments but it is quite difficult to configure a single
.travis.yml file which defines many Linux environments and some OS X
ones.
- It only runs Ubuntu 14.04 with an x86-64 CPU. This does not test
Android, ARM nor 32-bit x86 configurations.
- It only builds with glibc, not musl or other light C library.
Signed-off-by: Nicolas Iooss
Applied.
Thanks,
Jim
---
.travis.yml | 142
1 file changed, 142 insertions(+)
create mode 100644 .travis.yml
diff --git a/.travis.yml b/.travis.yml
new file mode 100644
index ..6dce35165bd3
--- /dev/null
+++ b/.travis.yml
@@ -0,0 +1,142 @@
+# Define the building environment
+language: c
+
+matrix:
+ fast_finish: true
+
+compiler:
+ - clang
+ - gcc
+
+env:
+ matrix:
+# Test the last version of Python and Ruby together, with some linkers
+- PYVER=python3.5 RUBYLIBVER=2.3
+- PYVER=python3.5 RUBYLIBVER=2.3 LINKER=gold
+- PYVER=python3.5 RUBYLIBVER=2.3 LINKER=bfd
+
+# Test several Python versions
+- PYVER=python2.7 RUBYLIBVER=2.3
+- PYVER=python3.3 RUBYLIBVER=2.3
+- PYVER=python3.4 RUBYLIBVER=2.3
+- PYVER=pypy RUBYLIBVER=2.3
+#- PYVER=pypy3 RUBYLIBVER=2.3 # PyPy3 5.5.0 provides the Python2 form of
some structures, which makes it incompatible with SWIG
+
+# Test several Ruby versions
+- PYVER=python3.5 RUBYLIBVER=2.0
+- PYVER=python3.5 RUBYLIBVER=2.1
+- PYVER=python3.5 RUBYLIBVER=2.2
+
+# Use Travis-CI Ubuntu 14.04 Trusty infrastructure
+sudo: required
+dist: trusty
+
+# Install SELinux userspace utilities dependencies
+addons:
+ apt:
+packages:
+- bison
+- flex
+- gawk
+- gettext
+- libaudit-dev
+- libbz2-dev
+- libcap-dev
+- libcap-ng-dev # This package is not whitelisted for the container
infrastructure (https://github.com/travis-ci/apt-package-whitelist/issues/1096)
+- libcunit1-dev
+- libdbus-glib-1-dev
+- libncurses5-dev
+- libpcre3-dev
+- patch
+- python3-dev
+- python-dev
+- swig
+- xmlto
+
+install:
+ # Download refpolicy Makefile for sepolgen tests
+ - sudo mkdir -p /usr/share/selinux/default
+ - sudo curl -o /usr/share/selinux/default/Makefile
'https://raw.githubusercontent.com/TresysTechnology/refpolicy/RELEASE_2_20170204/support/Makefile.devel'
+ - sudo sed "s,^PREFIX :=.*,PREFIX := $TRAVIS_BUILD_DIR/installdir/usr," -i
/usr/share/selinux/default/Makefile
+ - sudo mkdir -p /usr/share/selinux/refpolicy/include
+ - sudo curl -o /usr/share/selinux/refpolicy/include/build.conf
'https://raw.githubusercontent.com/TresysTechnology/refpolicy/RELEASE_2_20170204/build.conf'
+ - sudo mkdir -p /etc/selinux
+ - echo 'SELINUXTYPE=refpolicy' | sudo tee /etc/selinux/config
+
+ # Make sepolgen tests work without really installing anything in the real
root (doing this would conflict with Ubuntu packages)
+ - sed -e "s,\"\(/usr/bin/[cs]\),\"$TRAVIS_BUILD_DIR/installdir\1," -i
python/sepolgen/src/sepolgen/module.py
+
+before_script:
+ # clang on Travis-CI 14.04 environment is too old to support
-Wdouble-promotion
+ - if "$CC" --version |grep -q clang; then sed 's/ -Wdouble-promotion / /' -i
libselinux/src/Makefile libselinux/utils/Makefile ; fi
+
+ # Build and install in a temporary directory to run tests
+ - export DESTDIR="$TRAVIS_BUILD_DIR/installdir"
+
+ # Configure the variables for Python parts
+ - export VIRTUAL_ENV="$HOME/virtualenv/$PYVER"
+ - export PYTHON="$VIRTUAL_ENV/bin/python"
+ - export PYPREFIX="$($PYTHON -c 'import sys;print("python%d" %
sys.version_info[0])')"
+ # PyPy does not provide a config file for pkg-config nor a pypy-c.so
+ - if echo "$PYVER" | grep -q pypy ; then export PYINC=-I$($PYTHON -c 'import
sys;print(sys.prefix)')/include PYLIBS= ; fi
+ # Python virtualenvs do not support "import site;
print(site.getsitepackages()[0]"
+ # cf. https://github.com/pypa/virtualenv/issues/355#issuecomment-10250452
+ - export PYSITEDIR="$DESTDIR/usr/lib/$($PYTHON -c 'import sys;print("python%d.%d"
% sys.version_info[:2])')/site-packages"
+
+ # Find the Ruby executable with version $RUBYLIBVER
+ - export RUBY="$(ls -d -1 "$HOME/.rvm/rubies/ruby-$RUBYLIBVER"*/bin/ruby | head
-n 1)"
+
+ # Set the linker in $CC so that it gets used everywhere
+ - if [ -n "$LINKER" ]; then CC="$
Re: [PATCH] selinux: fix off-by-one in setprocattr
On Tue, 2017-02-07 at 15:30 -0800, Andy Lutomirski wrote:
> On Tue, Feb 7, 2017 at 2:43 PM, Paul Moore
> wrote:
> >
> > On Tue, Jan 31, 2017 at 11:54 AM, Stephen Smalley > v> wrote:
> > >
> > > SELinux tries to support setting/clearing of /proc/pid/attr
> > > attributes
> > > from the shell by ignoring terminating newlines and treating an
> > > attribute value that begins with a NUL or newline as an attempt
> > > to
> > > clear the attribute. However, the test for clearing attributes
> > > has
> > > always been wrong; it has an off-by-one error, and this could
> > > further
> > > lead to reading past the end of the allocated buffer since commit
> > > bb646cdb12e75d82258c2f2e7746d5952d3e321a ("proc_pid_attr_write():
> > > switch to memdup_user()"). Fix the off-by-one error.
> > >
> > > Even with this fix, setting and clearing /proc/pid/attr
> > > attributes
> > > from the shell is not straightforward since the interface does
> > > not
> > > support multiple write() calls (so shells that write the value
> > > and
> > > newline separately will set and then immediately clear the
> > > attribute,
> > > requiring use of echo -n to set the attribute), whereas trying to
> > > use
> > > echo -n "" to clear the attribute causes the shell to skip the
> > > write() call altogether since POSIX says that a zero-length write
> > > causes no side effects. Thus, one must use echo -n to set and
> > > echo
> > > without -n to clear, as in the following example:
> > > $ echo -n unconfined_u:object_r:user_home_t:s0 >
> > > /proc/$$/attr/fscreate
> > > $ cat /proc/$$/attr/fscreate
> > > unconfined_u:object_r:user_home_t:s0
> > > $ echo "" > /proc/$$/attr/fscreate
> > > $ cat /proc/$$/attr/fscreate
> > >
> > > Note the use of /proc/$$ rather than /proc/self, as otherwise
> > > the cat command will read its own attribute value, not that of
> > > the shell.
> > >
> > > There are no users of this facility to my knowledge; possibly we
> > > should just get rid of it.
>
> I'm not sure which facility you're referring to here, but setpriv(1)
> uses /proc/self/attr/current and /proc/self/attr/exec.
No, I just meant the weird hacks to support setting and clearing from
the shell, which has never been used to my knowledge. Setting and
clearing from programs, preferably via the libselinux helper functions,
has always been fine and is in widespread use.
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] selinux: fix off-by-one in setprocattr
On Tue, Feb 7, 2017 at 2:43 PM, Paul Moore wrote:
> On Tue, Jan 31, 2017 at 11:54 AM, Stephen Smalley wrote:
>> SELinux tries to support setting/clearing of /proc/pid/attr attributes
>> from the shell by ignoring terminating newlines and treating an
>> attribute value that begins with a NUL or newline as an attempt to
>> clear the attribute. However, the test for clearing attributes has
>> always been wrong; it has an off-by-one error, and this could further
>> lead to reading past the end of the allocated buffer since commit
>> bb646cdb12e75d82258c2f2e7746d5952d3e321a ("proc_pid_attr_write():
>> switch to memdup_user()"). Fix the off-by-one error.
>>
>> Even with this fix, setting and clearing /proc/pid/attr attributes
>> from the shell is not straightforward since the interface does not
>> support multiple write() calls (so shells that write the value and
>> newline separately will set and then immediately clear the attribute,
>> requiring use of echo -n to set the attribute), whereas trying to use
>> echo -n "" to clear the attribute causes the shell to skip the
>> write() call altogether since POSIX says that a zero-length write
>> causes no side effects. Thus, one must use echo -n to set and echo
>> without -n to clear, as in the following example:
>> $ echo -n unconfined_u:object_r:user_home_t:s0 > /proc/$$/attr/fscreate
>> $ cat /proc/$$/attr/fscreate
>> unconfined_u:object_r:user_home_t:s0
>> $ echo "" > /proc/$$/attr/fscreate
>> $ cat /proc/$$/attr/fscreate
>>
>> Note the use of /proc/$$ rather than /proc/self, as otherwise
>> the cat command will read its own attribute value, not that of the shell.
>>
>> There are no users of this facility to my knowledge; possibly we
>> should just get rid of it.
I'm not sure which facility you're referring to here, but setpriv(1)
uses /proc/self/attr/current and /proc/self/attr/exec.
--Andy
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
