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

[Pádraig Brady]

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.

thanks,
Pádraig.