Subject: bug#16335: closed (Re: bug#16335: Segmentation fault when using cp -a with SELinux and fakeroot)

From: [email-masked] (GNU bug Tracking System)
Date: Mon, 13 Jan 2014 14:51:02 +0000

Your bug report

#16335: Segmentation fault when using cp -a with SELinux and fakeroot

which was filed against the coreutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
16335: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16335
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#16335: Segmentation fault when using cp -a with SELinux and fakeroot
From: Pádraig Brady <[email-masked]>
To: Nicolas Iooss <[email-masked]>
Cc: [email-masked]
Date: Mon, 13 Jan 2014 14:50:13 +0000
List-Id: <debbugs-submit.debbugs.gnu.org>

I'm going to push the attached very soon, to address this.

thanks,
Pádraig.
>From 2b3b5bfcd5f4161d17c0bc3d43f6edcfc4a2b294 Mon Sep 17 00:00:00 2001
From: Nicolas Looss <[email-masked]>
Date: Sat, 4 Jan 2014 03:03:51 +0000
Subject: [PATCH] copy: fix a segfault in SELinux context copying code

* src/selinux.c (restorecon_private): On ArchLinux the
`fakeroot cp -a file1 file2` command segfaulted due
to getfscreatecon() returning a NULL context.
So map this to the sometimes ignored ENODATA error,
rather than crashing.
* tests/cp/no-ctx.sh: Add a new test case.
* tests/local.mk: Reference the new test.
* NEWS: Mention the fix.
Fixes http://bugs.gnu.org/16335
---
 NEWS               |    5 ++++
 src/selinux.c      |    5 ++++
 tests/cp/no-ctx.sh |   53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/local.mk     |    1 +
 4 files changed, 64 insertions(+), 0 deletions(-)
 create mode 100755 tests/cp/no-ctx.sh

diff --git a/NEWS b/NEWS
index 3e1f9c6..699a7d3 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,11 @@ GNU coreutils NEWS                                    -*- outline -*-
   the context of an existing directory to that of its last copied descendent.
   [bug introduced in coreutils-8.22]
 
+  cp -a, mv, and install --preserve-context, no longer seg fault when running
+  with SELinux enabled, when copying from file systems that return an error
+  when reading the SELinux context for a file.
+  [bug introduced in coreutils-8.22]
+
 
 * Noteworthy changes in release 8.22 (2013-12-13) [stable]
 
diff --git a/src/selinux.c b/src/selinux.c
index cd38a81..016db16 100644
--- a/src/selinux.c
+++ b/src/selinux.c
@@ -192,6 +192,11 @@ restorecon_private (char const *path, bool local)
     {
       if (getfscreatecon (&tcon) < 0)
         return rc;
+      if (!tcon)
+        {
+          errno = ENODATA;
+          return rc;
+        }
       rc = lsetfilecon (path, tcon);
       freecon (tcon);
       return rc;
diff --git a/tests/cp/no-ctx.sh b/tests/cp/no-ctx.sh
new file mode 100755
index 0000000..59d30de
--- /dev/null
+++ b/tests/cp/no-ctx.sh
@@ -0,0 +1,53 @@
+#!/bin/sh
+# Ensure we handle file systems returning no SELinux context,
+# which triggered a segmentation fault in coreutils-8.22.
+# This test is skipped on systems that lack LD_PRELOAD support; that's fine.
+# Similarly, on a system that lacks lgetfilecon altogether, skipping it is fine.
+
+# Copyright (C) 2014 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ cp
+require_gcc_shared_
+
+# Replace each getfilecon and lgetfilecon call with a call to these stubs.
+cat > k.c <<'EOF' || framework_failure_
+#include <selinux/selinux.h>
+#include <errno.h>
+
+int getfilecon (const char *path, security_context_t *con)
+{ errno=ENODATA; return -1; }
+int lgetfilecon (const char *path, security_context_t *con)
+{ errno=ENODATA; return -1; }
+EOF
+
+# Then compile/link it:
+$CC -shared -fPIC -O2 k.c -o k.so \
+  || framework_failure_ 'failed to build SELinux shared library'
+
+touch file_src
+
+# New file with SELinux context optionally included
+LD_PRELOAD=./k.so cp -a file_src file_dst || fail=1
+
+# Existing file with SELinux context optionally included
+LD_PRELOAD=./k.so cp -a file_src file_dst || fail=1
+
+# ENODATA should give an immediate error when required to preserve ctx
+# This is debatable, and maybe we should not fail when no context available?
+LD_PRELOAD=./k.so cp --preserve=context file_src file_dst && fail=1
+
+Exit $fail
diff --git a/tests/local.mk b/tests/local.mk
index dc7341c..9d556f6 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -161,6 +161,7 @@ all_tests =					\
   tests/rm/ext3-perf.sh				\
   tests/rm/cycle.sh				\
   tests/cp/link-heap.sh				\
+  tests/cp/no-ctx.sh				\
   tests/misc/tty-eof.pl				\
   tests/tail-2/inotify-hash-abuse.sh		\
   tests/tail-2/inotify-hash-abuse2.sh		\
-- 
1.7.7.6

Subject: Segmentation fault when using cp -a with SELinux and fakeroot
From: Nicolas Iooss <[email-masked]>
To: [email-masked]
Date: Fri, 03 Jan 2014 23:08:42 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

Hello,

After upgrading to coreutils 8.22 I can no longer build packages which
uses "cp -a" to copy files due to a segmentation fault happening in
libselinux.

I've tried to reproduce this bug with few commands, in a directory which
doesn't have any default context:

    $ mkdir /tmp/foobar
    $ matchpathcon
    /tmp/foobar	<<none>>
    $ touch /tmp/foobar/a
    $ fakeroot cp -a /tmp/foobar/a /tmp/foobar/b
    $ fakeroot cp -a /tmp/foobar/a /tmp/foobar/b
    /usr/bin/fakeroot: line 181:  9207 Segmentation fault

Without fakeroot there is no segmentation fault.

Even if the message says "/usr/bin/fakeroot", a coredump has been
created for cp. I've analyzed this dump using gdb and after some
debugging, I found out that restorecon_private (from src/selinux.c) was
calling lsetfilecon with a NULL security context which was obtained by
getfscreatecon (case "local = true" in the code [1]). This causes a null
pointer dereference in libselinux and so a SIGSEGV.

I've reported this bug to libselinux maintainers [2] and got the reply
that calling lsetfilecon with a NULL security context was like calling
strlen with a NULL string and that this was a problem in caller's code [3].

Hence I propose the attached patch to fix the segmentation fault. Could
you please accept it?

When you reply, please Cc me as I'm not subscribed.

Thanks,

Nicolas Iooss

-----------

System configuration during my tests:

* distro: ArchLinux which SELinux packages
* CPU arch: x86_64
* SELinux in permissive mode
* coreutils 8.22
* libselinux 2.2.1
* fakeroot 1.20

[1]
http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=blob;f=src/selinux.c;hb=v8.22#l191
[2] http://marc.info/?l=selinux&m=138763485330568&w=2
[3] http://marc.info/?l=selinux&m=138842015508829&w=2
>>From 2d9940379927d2536675a7970f16767e4d209f27 Mon Sep 17 00:00:00 2001
From: Nicolas Iooss <[email-masked]>
Date: Fri, 3 Jan 2014 22:47:17 +0100
Subject: [PATCH] Fix segmentation fault in restorecon_private

---
 src/selinux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/selinux.c b/src/selinux.c
index cd38a81..e4deba3 100644
--- a/src/selinux.c
+++ b/src/selinux.c
@@ -190,7 +190,7 @@ restorecon_private (char const *path, bool local)
 
   if (local)
     {
-      if (getfscreatecon (&tcon) < 0)
+      if (getfscreatecon (&tcon) < 0 || tcon == NULL)
         return rc;
       rc = lsetfilecon (path, tcon);
       freecon (tcon);
-- 
1.8.5.2