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

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

[Sumit Semwal]

Hi Kees, Andy,

On 15 June 2017 at 23:26, Sumit Semwal <sumit.semwal@linaro.org> 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?

Greatly appreciated!

Thanks and best regards,
Sumit

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

[Kees Cook]

On Thu, Jun 22, 2017 at 9:18 AM, Sumit Semwal <sumit.semwal@linaro.org> wrote:
> Hi Kees, Andy,
>
> On 15 June 2017 at 23:26, Sumit Semwal <sumit.semwal@linaro.org> 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?

I don't really want to have that change be a backport -- it's quite
invasive across multiple architectures.

I would say just add a kernel version check to the test. This is
probably not the only selftest that will need such things. :)

I'd be happy to review such changes!

-Kees

-- 
Kees Cook
Pixel Security

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

[Shuah Khan]

On 06/22/2017 10:53 AM, Kees Cook wrote:
> On Thu, Jun 22, 2017 at 9:18 AM, Sumit Semwal <sumit.semwal@linaro.org> wrote:
>> Hi Kees, Andy,
>>
>> On 15 June 2017 at 23:26, Sumit Semwal <sumit.semwal@linaro.org> 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?
> 
> I don't really want to have that change be a backport -- it's quite
> invasive across multiple architectures.
> 
> 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.

thanks,
-- Shuah

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

[Andy Lutomirski]

On Thu, Jun 22, 2017 at 10:09 AM, Shuah Khan <shuah@kernel.org> wrote:
> On 06/22/2017 10:53 AM, Kees Cook wrote:
>> On Thu, Jun 22, 2017 at 9:18 AM, Sumit Semwal <sumit.semwal@linaro.org> wrote:
>>> Hi Kees, Andy,
>>>
>>> On 15 June 2017 at 23:26, Sumit Semwal <sumit.semwal@linaro.org> 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?
>>
>> I don't really want to have that change be a backport -- it's quite
>> invasive across multiple architectures.
>>
>> 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.

--Andy

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

[Kees Cook]

On Thu, Jun 22, 2017 at 10:49 AM, Andy Lutomirski <luto@kernel.org> wrote:
> On Thu, Jun 22, 2017 at 10:09 AM, Shuah Khan <shuah@kernel.org> wrote:
>> On 06/22/2017 10:53 AM, Kees Cook wrote:
>>> On Thu, Jun 22, 2017 at 9:18 AM, Sumit Semwal <sumit.semwal@linaro.org> wrote:
>>>> Hi Kees, Andy,
>>>>
>>>> On 15 June 2017 at 23:26, Sumit Semwal <sumit.semwal@linaro.org> 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?
>>>
>>> I don't really want to have that change be a backport -- it's quite
>>> invasive across multiple architectures.
>>>
>>> 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.

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.

-Kees

-- 
Kees Cook
Pixel Security

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

[Shuah Khan]

On 06/22/2017 11:50 AM, Kees Cook wrote:
> On Thu, Jun 22, 2017 at 10:49 AM, Andy Lutomirski <luto@kernel.org> wrote:
>> On Thu, Jun 22, 2017 at 10:09 AM, Shuah Khan <shuah@kernel.org> wrote:
>>> On 06/22/2017 10:53 AM, Kees Cook wrote:
>>>> On Thu, Jun 22, 2017 at 9:18 AM, Sumit Semwal <sumit.semwal@linaro.org> wrote:
>>>>> Hi Kees, Andy,
>>>>>
>>>>> On 15 June 2017 at 23:26, Sumit Semwal <sumit.semwal@linaro.org> 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.

> 
> 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.

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 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?

3. Do you find testing with newer selftests to be useful?

thanks,
-- Shuah

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

[Tom Gall]

Hi

On Thu, Jun 22, 2017 at 2:06 PM, Shuah Khan <shuah@kernel.org> wrote:
> On 06/22/2017 11:50 AM, Kees Cook wrote:
>> On Thu, Jun 22, 2017 at 10:49 AM, Andy Lutomirski <luto@kernel.org> wrote:
>>> On Thu, Jun 22, 2017 at 10:09 AM, Shuah Khan <shuah@kernel.org> wrote:
>>>> On 06/22/2017 10:53 AM, Kees Cook wrote:
>>>>> On Thu, Jun 22, 2017 at 9:18 AM, Sumit Semwal <sumit.semwal@linaro.org> wrote:
>>>>>> Hi Kees, Andy,
>>>>>>
>>>>>> On 15 June 2017 at 23:26, Sumit Semwal <sumit.semwal@linaro.org> 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.

> 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


-- 
Regards,
Tom

Director, Linaro Mobile Group
Linaro.org │ Open source software for ARM SoCs
irc: tgall_foo | skype : tom_gall

"Where's the kaboom!? There was supposed to be an earth-shattering
kaboom!" Marvin Martian

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

[Shuah Khan]

Hi Tom,

On 06/22/2017 01:48 PM, Tom Gall wrote:
> Hi
> 
> On Thu, Jun 22, 2017 at 2:06 PM, Shuah Khan <shuah@kernel.org> wrote:
>> On 06/22/2017 11:50 AM, Kees Cook wrote:
>>> On Thu, Jun 22, 2017 at 10:49 AM, Andy Lutomirski <luto@kernel.org> wrote:
>>>> On Thu, Jun 22, 2017 at 10:09 AM, Shuah Khan <shuah@kernel.org> wrote:
>>>>> On 06/22/2017 10:53 AM, Kees Cook wrote:
>>>>>> On Thu, Jun 22, 2017 at 9:18 AM, Sumit Semwal <sumit.semwal@linaro.org> wrote:
>>>>>>> Hi Kees, Andy,
>>>>>>>
>>>>>>> On 15 June 2017 at 23:26, Sumit Semwal <sumit.semwal@linaro.org> 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.

The assumption that "the latest selftests provide more coverage, and
therefore should be better tests, even on older kernels." is incorrect.

Selftests in general track the kernel features. In some cases, new
tests could be added that provide better coverage on older kernels,
however, it is more likely that new tests are added to test new kernel
features and enhancements to existing features. Based on the second
"enhancements to existing features" it is more important to test newer
kernels with older selftests. This does happen in kernel integration
cycles during development.

As a general rule, testing stable kernels with their own selftests will
yield the best results.

> 
>>>
>>> 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?

Yes. That is the goal and that is what tests do. Tests do detect
dependencies on features, modules, config options and decide to skip
the test. If a test doesn't do that, it gets fixed.

> 
> 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.

Right. Please see above. However, correctly detecting dependencies
isn't possible in all cases. In some cases, fail is what it can do.
 
> 
> 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.
> 
>> 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.

Not necessarily the case.

> 
>>    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.

If the only reason is "shiny", I would say you might not be getting
the best results possible.

> 
>> 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.
> 

As I explained above, The assumption/perception that "the latest selftests
provide more coverage, and therefore should be better tests, even on older
kernels." is incorrect.

As per collecting data to see if testing newer selftests provide better
coverage or not might or might not be worth while exercise. Some releases
might include tests for existing features and some might not. The mix might
be different. As a general rule "selftests are intended to track and do track
features in their release" is a good assumption.

It might be useful to fix tests from newer releases so they "never fail" on
older releases might not give us the best ROI as whole. These need to be
evaluated case by case basis.

I would recommend the following approach based on this discussion and now
that we understand incorrect assumption and/or mis-perception to be the
basis for choosing to test stable kernels with selftests from new releases.

1. Testing stable kernels with their own selftests will yield the best
   results.
2. Testing stable kernels with newer selftests could be done if user finds
   that it provides better coverage, knowing that there is no guarantee that
   it will.

thanks,
-- Shuah

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

[Greg Kroah-Hartman]

On Thu, Jun 22, 2017 at 10:50:43AM -0700, Kees Cook wrote:
> On Thu, Jun 22, 2017 at 10:49 AM, Andy Lutomirski <luto@kernel.org> wrote:
> > On Thu, Jun 22, 2017 at 10:09 AM, Shuah Khan <shuah@kernel.org> wrote:
> >> On 06/22/2017 10:53 AM, Kees Cook wrote:
> >>> On Thu, Jun 22, 2017 at 9:18 AM, Sumit Semwal <sumit.semwal@linaro.org> wrote:
> >>>> Hi Kees, Andy,
> >>>>
> >>>> On 15 June 2017 at 23:26, Sumit Semwal <sumit.semwal@linaro.org> 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?
> >>>
> >>> I don't really want to have that change be a backport -- it's quite
> >>> invasive across multiple architectures.
> >>>
> >>> 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.
> 
> 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.

Well, ideally all new features added to the kernel should be able to be
detected by userspace somehow if they are present or not.

How do you expect a program to know if a feature has "failed" or is just
"not enabled/present in this kernel"?  Normally with syscalls this is
easy, same for sysfs changes.  Is seccomp in the bad state where there
is no way to detect the two different states here?  How is userspace
supposed to deal with that?

We make fun of glibc having a zillion crazy tests to determine kernel
features, and recently, just not wrapping new syscalls at all because
they are just frustrated at the compatibility issues over time.  Let's
not make their life any harder than it has to be please.

I don't see how any of the kselftest programs are any different than any
other userspace program that wants to use our kernel api, and as such,
any version of kselftest should be able to successfully run on any
kernel release.  If not, then we messed up in how we either wrote the
test, or how we added a new kernel api.  Neither is acceptable.

thanks,

greg k-h

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

[Andy Lutomirski]

On Thu, Jun 22, 2017 at 6:52 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, Jun 22, 2017 at 10:50:43AM -0700, Kees Cook wrote:
>> On Thu, Jun 22, 2017 at 10:49 AM, Andy Lutomirski <luto@kernel.org> wrote:
>> > On Thu, Jun 22, 2017 at 10:09 AM, Shuah Khan <shuah@kernel.org> wrote:
>> >> On 06/22/2017 10:53 AM, Kees Cook wrote:
>> >>> On Thu, Jun 22, 2017 at 9:18 AM, Sumit Semwal <sumit.semwal@linaro.org> wrote:
>> >>>> Hi Kees, Andy,
>> >>>>
>> >>>> On 15 June 2017 at 23:26, Sumit Semwal <sumit.semwal@linaro.org> 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?
>> >>>
>> >>> I don't really want to have that change be a backport -- it's quite
>> >>> invasive across multiple architectures.
>> >>>
>> >>> 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.
>>
>> 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.
>
> Well, ideally all new features added to the kernel should be able to be
> detected by userspace somehow if they are present or not.
>
> How do you expect a program to know if a feature has "failed" or is just
> "not enabled/present in this kernel"?  Normally with syscalls this is
> easy, same for sysfs changes.  Is seccomp in the bad state where there
> is no way to detect the two different states here?  How is userspace
> supposed to deal with that?
>
> We make fun of glibc having a zillion crazy tests to determine kernel
> features, and recently, just not wrapping new syscalls at all because
> they are just frustrated at the compatibility issues over time.  Let's
> not make their life any harder than it has to be please.
>
> I don't see how any of the kselftest programs are any different than any
> other userspace program that wants to use our kernel api, and as such,
> any version of kselftest should be able to successfully run on any
> kernel release.  If not, then we messed up in how we either wrote the
> test, or how we added a new kernel api.  Neither is acceptable.

That's a fair point.  We could add SECCOMP_GET_FEATURES that returns a
mask of otherwise-difficult-to-probe features.

Greg, for context, the issue here is that we made what was arguably a
design error in seccomp's interaction with ptrace.  After determining
that fixing it solved a bunch of problems and didn't break any user
programs, we fixed it.  There might be new code that relies on the fix
being present in the sense that it would be insecure without the fix.

The problem is that the fix is moderately intrusive and doesn't seem
like a great candidate for backporting, although we could plausibly do
it.

--Andy

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

[Sumit Semwal]

Hi Shuah,

On 23 June 2017 at 01:53, Shuah Khan <shuah@kernel.org> wrote:
> Hi Tom,
>
> On 06/22/2017 01:48 PM, Tom Gall wrote:
>> Hi
>>
>> On Thu, Jun 22, 2017 at 2:06 PM, Shuah Khan <shuah@kernel.org> wrote:
>>> On 06/22/2017 11:50 AM, Kees Cook wrote:
>>>> On Thu, Jun 22, 2017 at 10:49 AM, Andy Lutomirski <luto@kernel.org> wrote:
>>>>> On Thu, Jun 22, 2017 at 10:09 AM, Shuah Khan <shuah@kernel.org> wrote:
>>>>>> On 06/22/2017 10:53 AM, Kees Cook wrote:
>>>>>>> On Thu, Jun 22, 2017 at 9:18 AM, Sumit Semwal <sumit.semwal@linaro.org> wrote:
>>>>>>>> Hi Kees, Andy,
>>>>>>>>
>>>>>>>> On 15 June 2017 at 23:26, Sumit Semwal <sumit.semwal@linaro.org> 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.
>
> The assumption that "the latest selftests provide more coverage, and
> therefore should be better tests, even on older kernels." is incorrect.
>
> Selftests in general track the kernel features. In some cases, new
> tests could be added that provide better coverage on older kernels,
> however, it is more likely that new tests are added to test new kernel
> features and enhancements to existing features. Based on the second
> "enhancements to existing features" it is more important to test newer
> kernels with older selftests. This does happen in kernel integration
> cycles during development.
>
> As a general rule, testing stable kernels with their own selftests will
> yield the best results.
>
I would have agreed totally, if the selftests and the kernel were in
sync since forever. But since the kselftests are a comparatively
recent addition, the number of tests available for features existing
in LTS kernels is really quite small. Just as a comparison, 4.4-LTS
misses tests for bpf, cpufreq, gpio, media_tests, networking, prctl,
to name a few.

Also, while trying to run kselftests from later kernels with 4.4, we
only had a few failures for existing features, while most other tests
ran ok. Just another data point.

>>
>>>>
>>>> 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?
>
> Yes. That is the goal and that is what tests do. Tests do detect
> dependencies on features, modules, config options and decide to skip
> the test. If a test doesn't do that, it gets fixed.
>
>>
>> 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.
>
> Right. Please see above. However, correctly detecting dependencies
> isn't possible in all cases. In some cases, fail is what it can do.
>
>>
>> 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.
>>
>>> 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.
>
> Not necessarily the case.
>
>>
>>>    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.
>
> If the only reason is "shiny", I would say you might not be getting
> the best results possible.
>
>>
>>> 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.
>>
>
> As I explained above, The assumption/perception that "the latest selftests
> provide more coverage, and therefore should be better tests, even on older
> kernels." is incorrect.
>
> As per collecting data to see if testing newer selftests provide better
> coverage or not might or might not be worth while exercise. Some releases
> might include tests for existing features and some might not. The mix might
> be different. As a general rule "selftests are intended to track and do track
> features in their release" is a good assumption.
>
> It might be useful to fix tests from newer releases so they "never fail" on
> older releases might not give us the best ROI as whole. These need to be
> evaluated case by case basis.
>
> I would recommend the following approach based on this discussion and now
> that we understand incorrect assumption and/or mis-perception to be the
> basis for choosing to test stable kernels with selftests from new releases.
>
> 1. Testing stable kernels with their own selftests will yield the best
>    results.
> 2. Testing stable kernels with newer selftests could be done if user finds
>    that it provides better coverage, knowing that there is no guarantee that
>    it will.
>
> thanks,
> -- Shuah

Best,
Sumit.

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

[Kees Cook]

On Thu, Jun 22, 2017 at 7:40 PM, Andy Lutomirski <luto@kernel.org> wrote:
> On Thu, Jun 22, 2017 at 6:52 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>> On Thu, Jun 22, 2017 at 10:50:43AM -0700, Kees Cook wrote:
>>> On Thu, Jun 22, 2017 at 10:49 AM, Andy Lutomirski <luto@kernel.org> wrote:
>>> > On Thu, Jun 22, 2017 at 10:09 AM, Shuah Khan <shuah@kernel.org> wrote:
>>> >> On 06/22/2017 10:53 AM, Kees Cook wrote:
>>> >>> On Thu, Jun 22, 2017 at 9:18 AM, Sumit Semwal <sumit.semwal@linaro.org> wrote:
>>> >>>> Hi Kees, Andy,
>>> >>>>
>>> >>>> On 15 June 2017 at 23:26, Sumit Semwal <sumit.semwal@linaro.org> 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?
>>> >>>
>>> >>> I don't really want to have that change be a backport -- it's quite
>>> >>> invasive across multiple architectures.
>>> >>>
>>> >>> 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.
>>>
>>> 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.
>>
>> Well, ideally all new features added to the kernel should be able to be
>> detected by userspace somehow if they are present or not.
>>
>> How do you expect a program to know if a feature has "failed" or is just
>> "not enabled/present in this kernel"?  Normally with syscalls this is
>> easy, same for sysfs changes.  Is seccomp in the bad state where there
>> is no way to detect the two different states here?  How is userspace
>> supposed to deal with that?
>>
>> We make fun of glibc having a zillion crazy tests to determine kernel
>> features, and recently, just not wrapping new syscalls at all because
>> they are just frustrated at the compatibility issues over time.  Let's
>> not make their life any harder than it has to be please.
>>
>> I don't see how any of the kselftest programs are any different than any
>> other userspace program that wants to use our kernel api, and as such,
>> any version of kselftest should be able to successfully run on any
>> kernel release.  If not, then we messed up in how we either wrote the
>> test, or how we added a new kernel api.  Neither is acceptable.
>
> That's a fair point.  We could add SECCOMP_GET_FEATURES that returns a
> mask of otherwise-difficult-to-probe features.

But that's silly. The self tests includes all kinds of bug fix tests,
and adding each of those to some features list seems like crazy
overkill. And every API in the kernel is going to do this?

> Greg, for context, the issue here is that we made what was arguably a
> design error in seccomp's interaction with ptrace.  After determining
> that fixing it solved a bunch of problems and didn't break any user
> programs, we fixed it.  There might be new code that relies on the fix
> being present in the sense that it would be insecure without the fix.
>
> The problem is that the fix is moderately intrusive and doesn't seem
> like a great candidate for backporting, although we could plausibly do
> it.

Even if we did all this for seccomp, we're left with the general case.
This is not a situation unique to seccomp. Behaviors and bug fix tests
are added to selftests over time, and not all of those things are
backport-worthy. If a certain test is desired for a stable kernel, we
should backport the test.

-Kees

-- 
Kees Cook
Pixel Security

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

[Shuah Khan]

On 06/22/2017 10:02 PM, Sumit Semwal wrote:
> Hi Shuah,
> 
> On 23 June 2017 at 01:53, Shuah Khan <shuah@kernel.org> wrote:
>> Hi Tom,
>>
>> On 06/22/2017 01:48 PM, Tom Gall wrote:
>>> Hi
>>>
>>> On Thu, Jun 22, 2017 at 2:06 PM, Shuah Khan <shuah@kernel.org> wrote:
>>>> On 06/22/2017 11:50 AM, Kees Cook wrote:
>>>>> On Thu, Jun 22, 2017 at 10:49 AM, Andy Lutomirski <luto@kernel.org> wrote:
>>>>>> On Thu, Jun 22, 2017 at 10:09 AM, Shuah Khan <shuah@kernel.org> wrote:
>>>>>>> On 06/22/2017 10:53 AM, Kees Cook wrote:
>>>>>>>> On Thu, Jun 22, 2017 at 9:18 AM, Sumit Semwal <sumit.semwal@linaro.org> wrote:
>>>>>>>>> Hi Kees, Andy,
>>>>>>>>>
>>>>>>>>> On 15 June 2017 at 23:26, Sumit Semwal <sumit.semwal@linaro.org> 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.
>>
>> The assumption that "the latest selftests provide more coverage, and
>> therefore should be better tests, even on older kernels." is incorrect.
>>
>> Selftests in general track the kernel features. In some cases, new
>> tests could be added that provide better coverage on older kernels,
>> however, it is more likely that new tests are added to test new kernel
>> features and enhancements to existing features. Based on the second
>> "enhancements to existing features" it is more important to test newer
>> kernels with older selftests. This does happen in kernel integration
>> cycles during development.
>>
>> As a general rule, testing stable kernels with their own selftests will
>> yield the best results.
>>
> I would have agreed totally, if the selftests and the kernel were in
> sync since forever. But since the kselftests are a comparatively
> recent addition, the number of tests available for features existing
> in LTS kernels is really quite small. Just as a comparison, 4.4-LTS
> misses tests for bpf, cpufreq, gpio, media_tests, networking, prctl,
> to name a few.

Yes. Several tests have been added since 4.4. Unfortunately these new
additions don't qualify as candidates for stable releases.

> 
> Also, while trying to run kselftests from later kernels with 4.4, we
> only had a few failures for existing features, while most other tests
> ran ok. Just another data point.

That is good to hear. Let's fix the problems as you find them and make
the selftests from newer releases work well (if not perfect) on older
kernels.

4.13 is going to see TAP13 changes as well, so if selftests from newer
kernels work for you, you will see TAB13 benefits.

thanks,
-- Shuah

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@kernel.org> wrote:
>> On 06/22/2017 11:50 AM, Kees Cook wrote:
>>> On Thu, Jun 22, 2017 at 10:49 AM, Andy Lutomirski <luto@kernel.org> wrote:
>>>> On Thu, Jun 22, 2017 at 10:09 AM, Shuah Khan <shuah@kernel.org> wrote:
>>>>> On 06/22/2017 10:53 AM, Kees Cook wrote:
>>>>>> On Thu, Jun 22, 2017 at 9:18 AM, Sumit Semwal <sumit.semwal@linaro.org> wrote:
>>>>>>> Hi Kees, Andy,
>>>>>>>
>>>>>>> On 15 June 2017 at 23:26, Sumit Semwal <sumit.semwal@linaro.org> 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
> 
> 

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

[Tom Gall]

Hi Shuah,

On Fri, Jun 23, 2017 at 2:03 PM, Shuah Khan <shuah@kernel.org> wrote:
> On 06/22/2017 01:48 PM, Tom Gall wrote:
>> Hi
>>
>> On Thu, Jun 22, 2017 at 2:06 PM, Shuah Khan <shuah@kernel.org> wrote:
>>> On 06/22/2017 11:50 AM, Kees Cook wrote:
>>>> On Thu, Jun 22, 2017 at 10:49 AM, Andy Lutomirski <luto@kernel.org> wrote:
>>>>> On Thu, Jun 22, 2017 at 10:09 AM, Shuah Khan <shuah@kernel.org> wrote:
>>>>>> On 06/22/2017 10:53 AM, Kees Cook wrote:
>>>>>>> On Thu, Jun 22, 2017 at 9:18 AM, Sumit Semwal <sumit.semwal@linaro.org> wrote:
>>>>>>>> Hi Kees, Andy,
>>>>>>>>
>>>>>>>> On 15 June 2017 at 23:26, Sumit Semwal <sumit.semwal@linaro.org> 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.

My thought here wasn't to define the symbol, but to detect it.

If it's not defined, you know it's not an available feature and thus
avoid that portion of the test.

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

As Greg had mentioned, if we stay focused on features and function in
kernel, many tests shouldn't care what version of the kernel they are
on.

Likewise those tests that are for new function need to be
compartmentalized to be separate and not be mixed in in such a way
that a compile time failure cause parts of a test to not be built that
would be perfectly valid should the feature not be available.

> 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.

To me seeing a bunch of preventable ugly compile errors which don't
give guidance if they are stemming from invalid test cases seems like
something we should address. That sorta suggests there's some kind of
meta that decides to either build or not build a test based on feature
existence. And I really mean existence not kernel version.

That suggests the following rule, sefltests should always either
successfully build or not be built at all.

Anyway, I'm going to shut up until I have patches.

> -- 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
>>
>>
>



-- 
Regards,
Tom

Director, Linaro Mobile Group
Linaro.org │ Open source software for ARM SoCs
irc: tgall_foo | skype : tom_gall

"Where's the kaboom!? There was supposed to be an earth-shattering
kaboom!" Marvin Martian

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

[Luis R. Rodriguez]

On Thu, Jun 22, 2017 at 07:40:49PM -0700, Andy Lutomirski wrote:
> On Thu, Jun 22, 2017 at 6:52 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Thu, Jun 22, 2017 at 10:50:43AM -0700, Kees Cook wrote:
> >> On Thu, Jun 22, 2017 at 10:49 AM, Andy Lutomirski <luto@kernel.org> wrote:
> >> > On Thu, Jun 22, 2017 at 10:09 AM, Shuah Khan <shuah@kernel.org> wrote:
> >> >> On 06/22/2017 10:53 AM, Kees Cook wrote:
> >> 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.
> >
> > Well, ideally all new features added to the kernel should be able to be
> > detected by userspace somehow if they are present or not.
> >
> > How do you expect a program to know if a feature has "failed" or is just
> > "not enabled/present in this kernel"?  Normally with syscalls this is
> > easy, same for sysfs changes.  Is seccomp in the bad state where there
> > is no way to detect the two different states here?  How is userspace
> > supposed to deal with that?
> >
> > We make fun of glibc having a zillion crazy tests to determine kernel
> > features, and recently, just not wrapping new syscalls at all because
> > they are just frustrated at the compatibility issues over time.  Let's
> > not make their life any harder than it has to be please.
> >
> > I don't see how any of the kselftest programs are any different than any
> > other userspace program that wants to use our kernel api, and as such,
> > any version of kselftest should be able to successfully run on any
> > kernel release.  If not, then we messed up in how we either wrote the
> > test, or how we added a new kernel api.  Neither is acceptable.
> 
> That's a fair point.

I agreed with it as well just a few threads ago due to similar issues, however,
thinking this over I'm afraid this has some interesting side consequences for
fixes and what code goes upstream into kselftest.

<-- snip -->

> The problem is that the fix is moderately intrusive and doesn't seem
> like a great candidate for backporting, although we could plausibly do
> it.

Such is the case often actually.

So taking the position that any kselftest script on linux-next or a future
kernel should never break stable implicate that *any* fix going upstream for
which there is a respective ksefltest test *must* have a stable upstream fix.

Its not obvious to me that everyone is aware of this. What do we do about
those cases where we *don't* want a stable fix due to the complexity?

  Luis

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

[Greg Kroah-Hartman]

On Thu, Jun 22, 2017 at 07:40:49PM -0700, Andy Lutomirski wrote:
> Greg, for context, the issue here is that we made what was arguably a
> design error in seccomp's interaction with ptrace.  After determining
> that fixing it solved a bunch of problems and didn't break any user
> programs, we fixed it.  There might be new code that relies on the fix
> being present in the sense that it would be insecure without the fix.
> 
> The problem is that the fix is moderately intrusive and doesn't seem
> like a great candidate for backporting, although we could plausibly do
> it.

That's fine, not all bugfixes that tests are created to find, should be
backported.  That's up to the stable maintainers, or someone who has a
device/vendor tree based on that kernel if they want to do that or not.

That has nothing to do with the fact that the test should fail or
gracefully degrade.  Tests should fail if the action that they are
testing fails.  They should degrade and not run if the _feature_ they
are testing is not present.

Yes, sometimes this is hard, like with the seccomp stuff, and will not
always work, but that's the rule for our userspace api independant of
any testing framework or code.

Look at xfstests, no one gets mad when it adds a new test that old
kernels fail at.  It's up to someone else to either backport the kernel
change, if they want it fixed in an old kernel, not to have xfstests
just not run it at all!  There's nothing different here either.

thanks,

greg k-h

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

[Greg Kroah-Hartman]

On Sat, Jun 24, 2017 at 02:34:07AM +0200, Luis R. Rodriguez wrote:
> So taking the position that any kselftest script on linux-next or a future
> kernel should never break stable implicate that *any* fix going upstream for
> which there is a respective ksefltest test *must* have a stable upstream fix.

What, no!  If it's a bug, that kselftest points out, great, it's up to
stable or a vendor to backport that fix if they want it.

Again, it's just like any other test suite, look at xfstests, no one
gets mad when it adds new tests that fail on old kernels, due to bugs
there, right?

> Its not obvious to me that everyone is aware of this. What do we do about
> those cases where we *don't* want a stable fix due to the complexity?

That's up to the stable maintainers or the vendors that maintain their
own kernel trees.

thanks,

greg k-h

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

[Luis R. Rodriguez]

On Sat, Jun 24, 2017 at 06:45:37AM +0200, Greg Kroah-Hartman wrote:
> On Sat, Jun 24, 2017 at 02:34:07AM +0200, Luis R. Rodriguez wrote:
> > So taking the position that any kselftest script on linux-next or a future
> > kernel should never break stable implicate that *any* fix going upstream for
> > which there is a respective ksefltest test *must* have a stable upstream fix.
> 
> What, no!  If it's a bug, that kselftest points out, great, it's up to
> stable or a vendor to backport that fix if they want it.

Makes sense!

> Again, it's just like any other test suite, look at xfstests, no one
> gets mad when it adds new tests that fail on old kernels, due to bugs
> there, right?

Correct but that means there is an information gap of which test cases fit into
this category. An option to run kselftest with the ability to only run tests
designed to also include stable would be good, otherwise Sumit Semwal or others
would be sending reports or issues for things folks already designed the fix to
*not* be a stable candidate. This actually would also prove useful to
distributions to ensure to then run kselftest as part of their distribution
test suite with all bells and whistles enabled so that they can make a
determination of what questionable "fixes" things to fold in somehow.

Unless of course we want these discussions to purposely bubble up as an
alternative to this kselftest option. It seems rather wasteful though.

> > Its not obvious to me that everyone is aware of this. What do we do about
> > those cases where we *don't* want a stable fix due to the complexity?
> 
> That's up to the stable maintainers or the vendors that maintain their
> own kernel trees.

But if the above is already decided having folks send emails about it
seems pointless.

  Luis

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

[Sumit Semwal]

Hi Andy,

On 24 June 2017 at 10:13, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> On Thu, Jun 22, 2017 at 07:40:49PM -0700, Andy Lutomirski wrote:
>> Greg, for context, the issue here is that we made what was arguably a
>> design error in seccomp's interaction with ptrace.  After determining
>> that fixing it solved a bunch of problems and didn't break any user
>> programs, we fixed it.  There might be new code that relies on the fix
>> being present in the sense that it would be insecure without the fix.
>>
>> The problem is that the fix is moderately intrusive and doesn't seem
>> like a great candidate for backporting, although we could plausibly do
>> it.
>
> That's fine, not all bugfixes that tests are created to find, should be
> backported.  That's up to the stable maintainers, or someone who has a
> device/vendor tree based on that kernel if they want to do that or not.
>
> That has nothing to do with the fact that the test should fail or
> gracefully degrade.  Tests should fail if the action that they are
> testing fails.  They should degrade and not run if the _feature_ they
> are testing is not present.

So, any updates on this yet - getting the seccomp tests to degrade
gracefully? I realise you mentioned that the fix could be intrusive;
just wanted to know if it was on your radar still.
>
> Yes, sometimes this is hard, like with the seccomp stuff, and will not
> always work, but that's the rule for our userspace api independant of
> any testing framework or code.
>
> Look at xfstests, no one gets mad when it adds a new test that old
> kernels fail at.  It's up to someone else to either backport the kernel
> change, if they want it fixed in an old kernel, not to have xfstests
> just not run it at all!  There's nothing different here either.
>
> thanks,
>
> greg k-h

Thanks much,
Sumit.