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

[Pádraig Brady]

On 01/04/2014 01:42 AM, Pádraig Brady wrote:
> On 01/03/2014 10:08 PM, Nicolas Iooss wrote:
>> 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
> 
> Thanks for the very thorough analysis and patch.
> The patch looks correct as getfscreatecon() is
> documented to return a NULL context in some cases.
> I'll see if I can add a robust test and will apply
> this in your name.

Actually what's errno set to with tcon is NULL.
If if was 0 you might get the classic "error success" message
if using the --preserve=context option rather than -a for example.

I.E. the following might be more appropriate.
Note neither Fedora 15 or 20 here produce a NULL value with fakeroot.

thanks,
Pádraig.

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;