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

From: Pádraig Brady <[email-masked]>
Date: Tue, 14 Jan 2014 13:55:22 +0000

On 01/14/2014 12:38 PM, Bernhard Voelker wrote:
> On 01/14/2014 12:35 PM, Pádraig Brady wrote:
>> On 01/14/2014 10:54 AM, Bernhard Voelker wrote:
>>> + test -e preloaded
>>> + skip_ 'LD_PRELOAD interception failed'
>>
>> Oh right. I think this should restrict the test appropriately...
>>
>> commit 3620df245a2211dc441e019845f98b91333bda77
>> Author: Pádraig Brady <[email-masked]>
>> Date:   Tue Jan 14 11:30:51 2014 +0000
>>
>>     tests: restrict a recent SELinux test to SELinux systems
>>
>>     * tests/cp/no-ctx.sh: Since the test diagnoses whether the
>>     intercepted lgetfilecon() calls are actually called or not,
> 
> The witness file is only created for getfilecon() - not for
> lgetfilecon().

In the wrapper, lgetfilecon() calls getfilecon() ?

>>     restrict the test to systems where that occurs.
>>     The test cases are minimal on non SELinux systems and should
>>     be well covered by other tests.
>>     Reported-by: Bernhard Voelker
>>
>> diff --git a/tests/cp/no-ctx.sh b/tests/cp/no-ctx.sh
>> index 3b5eb82..6851785 100755
>> --- a/tests/cp/no-ctx.sh
>> +++ b/tests/cp/no-ctx.sh
>> @@ -22,6 +22,7 @@
>>  . "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
>>  print_ver_ cp
>>  require_gcc_shared_
>> +requires_selinux_

BTW that should be require_selinux_
It's dangerous that we don't diagnose such typos.
I wonder would it be appropriate to have a test_require_()
wrapper that would catch such things, and be called like:
  test_require_ gcc_shared selinux

>>  # Replace each getfilecon and lgetfilecon call with a call to these stubs.
>>  cat > k.c <<'EOF' || framework_failure_
> 
> I'm a bit biased about this patch. Okay, it's perfectly valid to
> skip the test if the system doesn't support SELinux, but OTOH it may
> be quite valuable to verify the exit codes like that on non-SELinux
> systems,

Well I did state that "The test cases are minimal on non SELinux systems
and should be well covered by other tests"...

> i.e., based on stderr of the last cp call, the "preloaded"
> file must exist or not. The test could verify that. WDYT?

...and if the last cp fails it could be due to the wrapper running,
or SELinux not being supported. We'd need something else to
distinguish here, and require_selinux_ is the best I can think of
at present.

I suppose an alternative would be to refactor require_selinux_
to a function that just determines if it's available and do:

test -e preloaded ||
  { have_selinux_ && framework_failure_ 'LD_PRELOAD interception failed'; }

thanks,
Pádraig.