Re: seccomp ptrace selftest failures with 4.4-stable [Was: Re: LTS testing with latest kselftests - some failures]

[Shuah Khan]

On 06/22/2017 01:48 PM, Tom Gall wrote:
> Hi
> 
> On Thu, Jun 22, 2017 at 2:06 PM, Shuah Khan <shuah@xxxxxxxxxx> wrote:
>> On 06/22/2017 11:50 AM, Kees Cook wrote:
>>> On Thu, Jun 22, 2017 at 10:49 AM, Andy Lutomirski <luto@xxxxxxxxxx> wrote:
>>>> On Thu, Jun 22, 2017 at 10:09 AM, Shuah Khan <shuah@xxxxxxxxxx> wrote:
>>>>> On 06/22/2017 10:53 AM, Kees Cook wrote:
>>>>>> On Thu, Jun 22, 2017 at 9:18 AM, Sumit Semwal <sumit.semwal@xxxxxxxxxx> wrote:
>>>>>>> Hi Kees, Andy,
>>>>>>>
>>>>>>> On 15 June 2017 at 23:26, Sumit Semwal <sumit.semwal@xxxxxxxxxx> wrote:
>>>>>>>> 3. 'seccomp ptrace hole closure' patches got added in 4.7 [3] -
>>>>>>>> feature and test together.
>>>>>>>> - This one also seems like a security hole being closed, and the
>>>>>>>> 'feature' could be a candidate for stable backports, but Arnd tried
>>>>>>>> that, and it was quite non-trivial. So perhaps  we'll need some help
>>>>>>>> from the subsystem developers here.
>>>>>>>
>>>>>>> Could you please help us sort this out? Our goal is to help Greg with
>>>>>>> testing stable kernels, and currently the seccomp tests fail due to
>>>>>>> missing feature (seccomp ptrace hole closure) getting tested via
>>>>>>> latest kselftest.
>>>>>>>
>>>>>>> If you feel the feature isn't a stable candidate, then could you
>>>>>>> please help make the test degrade gracefully in its absence?
>>
>> In some cases, it is not easy to degrade and/or check for a feature.
>> Probably several security features could fall in this bucket.
>>
>>>>>>
>>>>>> I don't really want to have that change be a backport -- it's quite
>>>>>> invasive across multiple architectures.
>>
>> Agreed. The same test for kernel applies to tests as well. If a kernel
>> feature can't be back-ported, the test for that feature will fall in the
>> same bucket. It shouldn't be back-ported.
>>
>>>>>>
>>>>>> I would say just add a kernel version check to the test. This is
>>>>>> probably not the only selftest that will need such things. :)
>>>>>
>>>>> Adding release checks to selftests is going to problematic for maintenance.
>>>>> Tests should fail gracefully if feature isn't supported in older kernels.
>>>>>
>>>>> Several tests do that now and please find a way to check for dependencies
>>>>> and feature availability and fail the test gracefully. If there is a test
>>>>> that can't do that for some reason, we can discuss it, but as a general
>>>>> rule, I don't want to see kselftest patches that check release.
>>>>
>>>> If a future kernel inadvertently loses the new feature and degrades to
>>>> the behavior of old kernels, that would be a serious bug and should be
>>>> caught.
>>
>> Agreed. If I understand you correctly, by not testing stable kernels
>> with their own selftests, some serious bugs could go undetected.
> 
> Personally I'm a bit skeptical. I think the reasoning is more that the
> latest selftests provide more coverage, and therefore should be better
> tests, even on older kernels.
> 
>>>
>>> Right. I really think stable kernels should be tested with their own
>>> selftests. If some test is needed in a stable kernel it should be
>>> backported to that stable kernel.
>>
>> Correct. This is always a safe option. There might be cases that even
>> prevent tests being built, especially if a new feature adds new fields
>> to an existing structure.
>>
>> It appears in some cases, users want to run newer tests on older kernels.
>> Some tests can clearly detect feature support using module presence and/or
>> Kconfig enabled or disabled. These are conditions even on a kernel that
>> supports a new module or new config option. The kernel the test is running
>> on might not have the feature enabled or module might not be present. In
>> these cases, it would be easier to detect and skip the test.
>>
>> However, some features aren't so easy. For example:
>>
>> - a new flag is added to a syscall, and new test is added. It might not
>>   be easy to detect that.
>> - We might have some tests that can't detect and skip.
>>
>> Based on this discussion, it is probably accurate to say:
>>
>> 1. It is recommended that selftests from the same release be run on the
>>    kernel.
>> 2. Selftests from newer kernels will run on older kernels, user should
>>    understand the risks such as some tests might fail and might not
>>    detect feature degradation related bugs.
>> 3. Selftests will fail gracefully on older releases if at all possible.
> 
> How about gracefully be skipped instead of fail?
> 
> The later suggests the test case in some situations can detect it's
> pointless to run something and say as much instead of emitting a
> failure that would be a waste of time to look into.
> 
> As another example take tools/testing/selftests/net/psock_fanout.c
> On 4.9 it'll fail to compile (using master's selftests) because
> PACKET_FANOUT_FLAG_UNIQUEID isn't defined. Add a simple #ifdef for
> that symbol and the psock_fanout test will compile and run just fine.

Unfortunately adding PACKET_FANOUT_FLAG_UNIQUEID isn't correct. This is
a new feature that went into 4.12. You don't want to define this for older
kernel which will break on newer kernels.

This is one concern I have that in am attempt to fix mainline selftest to
be able to test older kernels will cause problems.

This is a good example of a new test that has dependency on a new define
in an include file which will be hard to check - compile will fail on
older kernels. The rest of the net tests will compile and run.

-- Shuah

>
>> Sumit!
>>
>> 1. What are the reasons for testing older kernel with selftests from
>>    newer kernels? What are the benefits you see for doing so?
> 
> I think the presumption is the latest greatest collection of selftests
> are the best, most complete.
> 
>>    I am looking to understand the need/reasons for this use-case. In our
>>    previous discussion on this subject, I did say, you should be able to
>>    do so with some exceptions.
>>
>> 2. Do you test kernels with the selftests from the same release?
> 
> We have the ability to do either. The new shiny .... it calls.
> 
>> 3. Do you find testing with newer selftests to be useful?
> 
> I think it comes down to coverage and again the current perception
> that latest greatest is better. Quantitatively we haven't collected
> data to support that position tho it would be interesting to compare
> say a 4.4-lts and it's selftests directory to a mainline, see how much
> was new and then find out how much of those new selftests actually
> work on the older 4.4-lts.
> 
>> thanks,
>> -- Shuah
> 
>