Seccomp implications for glibc wrapper function changes

Seccomp implications for glibc wrapper function changes

[Michael Kerrisk (man-pages)]

Hello,

I was recently testing some code I'd written a while back that makes
use of seccomp filters to control which system calls a process can
make, and I got a surpise when someone showed the code no longer
worked in on a system that had glibc 2.26.

The behavior change resulted from Adhemerval's glibc commit

     commit b41152d716ee9c5ba34495a54e64ea2b732139b5
     Author: Adhemerval Zanella <adhemerval.zanella-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
     Date:   Fri Nov 11 15:00:03 2016 -0200

        Consolidate Linux open implementation
            [...]
            3. Use __NR_openat as default syscall for open{64}.

The commit in question changed the glibc open() wrapper to swtcch from
use the kernel's open() system call to using the kernel's openat()
system call.

This change broke my code that was doing seccomp filtering for the
open() system call number (__NR_open). The breakage in question is not
serious, since this was really just demonstration code. However, I
want to raise awareness that these sorts of changes have the potential
to possibly cause breakages for some code using seccomp, and note that
I think such changes should not be made lightly or gratuitously. (In
the above commit, it's not clear why the switch was made to using
openat(): there's no mention of the reasoning in the commit message,
nor is there anything that is obvious from reading through the code
change itself.)

Best regards,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: Seccomp implications for glibc wrapper function changes

[Adhemerval Zanella]

On 07/11/2017 18:35, Michael Kerrisk (man-pages) wrote:
> Hello,
> 
> I was recently testing some code I'd written a while back that makes
> use of seccomp filters to control which system calls a process can
> make, and I got a surpise when someone showed the code no longer
> worked in on a system that had glibc 2.26.
> 
> The behavior change resulted from Adhemerval's glibc commit
> 
>      commit b41152d716ee9c5ba34495a54e64ea2b732139b5
>      Author: Adhemerval Zanella <adhemerval.zanella-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>      Date:   Fri Nov 11 15:00:03 2016 -0200
> 
>         Consolidate Linux open implementation
>             [...]
>             3. Use __NR_openat as default syscall for open{64}.
> 
> The commit in question changed the glibc open() wrapper to swtcch from
> use the kernel's open() system call to using the kernel's openat()
> system call.
> 
> This change broke my code that was doing seccomp filtering for the
> open() system call number (__NR_open). The breakage in question is not
> serious, since this was really just demonstration code. However, I
> want to raise awareness that these sorts of changes have the potential
> to possibly cause breakages for some code using seccomp, and note that
> I think such changes should not be made lightly or gratuitously. (In
> the above commit, it's not clear why the switch was made to using
> openat(): there's no mention of the reasoning in the commit message,
> nor is there anything that is obvious from reading through the code
> change itself.)

Your code would 'break' if you run with on a new architecture that does
not implement __NR_open, which it is the default for new architecture
on Linux.

In fact I hardly consider this is a 'break' since the user API we
export does not have any constraint which underlying syscall we use.
For instance, a user can seccomp gettimeofday syscall on a system
without vDSO just to found out it is 'broken' on a vDSO kernel.

I think we should not constraint for this specific usercase; if one
is doing syscall filtering it is expected system level knowledge to
handle all possible syscalls related.  For instance, I would expect
that if the idea is to filtering open() libc implementation the
program should also filter __NR_openat and __NR_openat2 since it
is semantically possible to implement open() with __NR_openat2 if
the syscall is available.

Now for the reasoning of using __NR_openat is since we have support
for it on all architecture it meant less logic to handle possible
architecture differences. 

Re: Seccomp implications for glibc wrapper function changes

[Michael Kerrisk (man-pages)]

Hi Adhemerval

On 7 November 2017 at 22:14, Adhemerval Zanella
<adhemerval.zanella-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>
>
> On 07/11/2017 18:35, Michael Kerrisk (man-pages) wrote:
>> Hello,
>>
>> I was recently testing some code I'd written a while back that makes
>> use of seccomp filters to control which system calls a process can
>> make, and I got a surpise when someone showed the code no longer
>> worked in on a system that had glibc 2.26.
>>
>> The behavior change resulted from Adhemerval's glibc commit
>>
>>      commit b41152d716ee9c5ba34495a54e64ea2b732139b5
>>      Author: Adhemerval Zanella <adhemerval.zanella-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>      Date:   Fri Nov 11 15:00:03 2016 -0200
>>
>>         Consolidate Linux open implementation
>>             [...]
>>             3. Use __NR_openat as default syscall for open{64}.
>>
>> The commit in question changed the glibc open() wrapper to swtcch from
>> use the kernel's open() system call to using the kernel's openat()
>> system call.
>>
>> This change broke my code that was doing seccomp filtering for the
>> open() system call number (__NR_open). The breakage in question is not
>> serious, since this was really just demonstration code. However, I
>> want to raise awareness that these sorts of changes have the potential
>> to possibly cause breakages for some code using seccomp, and note that
>> I think such changes should not be made lightly or gratuitously. (In
>> the above commit, it's not clear why the switch was made to using
>> openat(): there's no mention of the reasoning in the commit message,
>> nor is there anything that is obvious from reading through the code
>> change itself.)
>
> Your code would 'break' if you run with on a new architecture that does
> not implement __NR_open, which it is the default for new architecture
> on Linux.

(Is it the default for new architectures? I was unaware of that.)

> In fact I hardly consider this is a 'break' since the user API we
> export does not have any constraint which underlying syscall we use.
> For instance, a user can seccomp gettimeofday syscall on a system
> without vDSO just to found out it is 'broken' on a vDSO kernel.
>
> I think we should not constraint for this specific usercase; if one
> is doing syscall filtering it is expected system level knowledge to
> handle all possible syscalls related.  For instance, I would expect
> that if the idea is to filtering open() libc implementation the
> program should also filter __NR_openat and __NR_openat2 since it
> is semantically possible to implement open() with __NR_openat2 if
> the syscall is available.

Perhaps my use of the word 'break' was a bit too loaded. (And if my
mail came across as somehow critical of your patch, that was not my
intention, and if you feel it as such, I do apologize for my poor
wording.)

My "broken" code was just some demo code, and I got surprised by the
behavior change. And I agree with pretty much everything you say,
especially the point about needing good system level knowledge when
you are doing system call filtering (and of course a real-world
application that cared about filtering open() should also be filtering
for openat()). This is one of the reasons that Linux man-pages have
steadily been acquiring subsections called "C library/kernel
differences". But I did just want to raise some awareness about that
these sorts of changes could be a possible issue in some cases.

(By the way, unless I am missing something, there is no __NR_openat2?)

> Now for the reasoning of using __NR_openat is since we have support
> for it on all architecture it meant less logic to handle possible
> architecture differences.

Fair enough. Maybe this could be stated more explicitly in the commit
message though?

With best regards,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: Seccomp implications for glibc wrapper function changes

[Adhemerval Zanella]

On 07/11/2017 19:47, Michael Kerrisk (man-pages) wrote:
> Hi Adhemerval
> 
> On 7 November 2017 at 22:14, Adhemerval Zanella
> <adhemerval.zanella-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>
>>
>> On 07/11/2017 18:35, Michael Kerrisk (man-pages) wrote:
>>> Hello,
>>>
>>> I was recently testing some code I'd written a while back that makes
>>> use of seccomp filters to control which system calls a process can
>>> make, and I got a surpise when someone showed the code no longer
>>> worked in on a system that had glibc 2.26.
>>>
>>> The behavior change resulted from Adhemerval's glibc commit
>>>
>>>      commit b41152d716ee9c5ba34495a54e64ea2b732139b5
>>>      Author: Adhemerval Zanella <adhemerval.zanella-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>      Date:   Fri Nov 11 15:00:03 2016 -0200
>>>
>>>         Consolidate Linux open implementation
>>>             [...]
>>>             3. Use __NR_openat as default syscall for open{64}.
>>>
>>> The commit in question changed the glibc open() wrapper to swtcch from
>>> use the kernel's open() system call to using the kernel's openat()
>>> system call.
>>>
>>> This change broke my code that was doing seccomp filtering for the
>>> open() system call number (__NR_open). The breakage in question is not
>>> serious, since this was really just demonstration code. However, I
>>> want to raise awareness that these sorts of changes have the potential
>>> to possibly cause breakages for some code using seccomp, and note that
>>> I think such changes should not be made lightly or gratuitously. (In
>>> the above commit, it's not clear why the switch was made to using
>>> openat(): there's no mention of the reasoning in the commit message,
>>> nor is there anything that is obvious from reading through the code
>>> change itself.)
>>
>> Your code would 'break' if you run with on a new architecture that does
>> not implement __NR_open, which it is the default for new architecture
>> on Linux.
> 
> (Is it the default for new architectures? I was unaware of that.)

Yes, new architectures should use the kernel headers
include/uapi/asm-generic/unistd.h for syscall definition and the idea
is to avoid duplication of functionality like syscalls that supplant
each one (openat and open for instance).  AArch64, for instance, only
supports on compat more for 32 bits.

> 
>> In fact I hardly consider this is a 'break' since the user API we
>> export does not have any constraint which underlying syscall we use.
>> For instance, a user can seccomp gettimeofday syscall on a system
>> without vDSO just to found out it is 'broken' on a vDSO kernel.
>>
>> I think we should not constraint for this specific usercase; if one
>> is doing syscall filtering it is expected system level knowledge to
>> handle all possible syscalls related.  For instance, I would expect
>> that if the idea is to filtering open() libc implementation the
>> program should also filter __NR_openat and __NR_openat2 since it
>> is semantically possible to implement open() with __NR_openat2 if
>> the syscall is available.
> 
> Perhaps my use of the word 'break' was a bit too loaded. (And if my
> mail came across as somehow critical of your patch, that was not my
> intention, and if you feel it as such, I do apologize for my poor
> wording.)

In fact I did not take is a criticize, but rather I was worried users
would expect that kind of syscalls stability over glibc releases.  The
'break' word indeed raise a flag ;)

> 
> My "broken" code was just some demo code, and I got surprised by the
> behavior change. And I agree with pretty much everything you say,
> especially the point about needing good system level knowledge when
> you are doing system call filtering (and of course a real-world
> application that cared about filtering open() should also be filtering
> for openat()). This is one of the reasons that Linux man-pages have
> steadily been acquiring subsections called "C library/kernel
> differences". But I did just want to raise some awareness about that
> these sorts of changes could be a possible issue in some cases.

I would expect a more high level library such libseccomp to provide
family like filter (to get all variant of open, rename, etc.).

> 
> (By the way, unless I am missing something, there is no __NR_openat2?)

There is not indeed, I confused with rename which has 3 variants
(__NR_rename, __NR_renameat, and __NR_renameat2).

> 
>> Now for the reasoning of using __NR_openat is since we have support
>> for it on all architecture it meant less logic to handle possible
>> architecture differences.
> 
> Fair enough. Maybe this could be stated more explicitly in the commit
> message though?

I assumed it was implicit by the idea of consolidation. But I agree
a more thoughtfully explanation on commit won't hurt.

Re: Seccomp implications for glibc wrapper function changes

[Florian Weimer]

On 11/07/2017 09:35 PM, Michael Kerrisk (man-pages) wrote:
> This change broke my code that was doing seccomp filtering for the
> open() system call number (__NR_open). The breakage in question is not
> serious, since this was really just demonstration code. However, I
> want to raise awareness that these sorts of changes have the potential
> to possibly cause breakages for some code using seccomp, and note that
> I think such changes should not be made lightly or gratuitously.

I have the opposite view: We should make such changes as often as 
possible, to remind people that seccomp filters (and certain SELinux and 
AppArmor policies) are incompatible with the GNU/Linux model, where 
everything is developed separately and not maintained within a single 
source tree (unlike say OpenBSD).  This means that you really can't 
deviate from the upstream Linux userspace ABI (in the broadest possible 
sense) and still expect things to work.

I know that people like to slap seccomp filters on everything today, but 
without careful examination, that is likely to introduce bugs 
(particularly on rarely used code paths).  It can also cause the process 
to switch to legacy interfaces with known issues (e.g., reading from 
/dev/urandom instead of getrandom, without waiting for the kernel to 
signal initialization of the pool).

Thanks,
Florian

Re: Seccomp implications for glibc wrapper function changes

[Michael Kerrisk (man-pages)]

Hello Adhemerval

On 8 November 2017 at 02:18, Adhemerval Zanella
<adhemerval.zanella-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>
>
> On 07/11/2017 19:47, Michael Kerrisk (man-pages) wrote:
>> Hi Adhemerval
>>
>> On 7 November 2017 at 22:14, Adhemerval Zanella
>> <adhemerval.zanella-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>
>>>
>>> On 07/11/2017 18:35, Michael Kerrisk (man-pages) wrote:
>>>> Hello,
>>>>
>>>> I was recently testing some code I'd written a while back that makes
>>>> use of seccomp filters to control which system calls a process can
>>>> make, and I got a surpise when someone showed the code no longer
>>>> worked in on a system that had glibc 2.26.
>>>>
>>>> The behavior change resulted from Adhemerval's glibc commit
>>>>
>>>>      commit b41152d716ee9c5ba34495a54e64ea2b732139b5
>>>>      Author: Adhemerval Zanella <adhemerval.zanella-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>>      Date:   Fri Nov 11 15:00:03 2016 -0200
>>>>
>>>>         Consolidate Linux open implementation
>>>>             [...]
>>>>             3. Use __NR_openat as default syscall for open{64}.
>>>>
>>>> The commit in question changed the glibc open() wrapper to swtcch from
>>>> use the kernel's open() system call to using the kernel's openat()
>>>> system call.
>>>>
>>>> This change broke my code that was doing seccomp filtering for the
>>>> open() system call number (__NR_open). The breakage in question is not
>>>> serious, since this was really just demonstration code. However, I
>>>> want to raise awareness that these sorts of changes have the potential
>>>> to possibly cause breakages for some code using seccomp, and note that
>>>> I think such changes should not be made lightly or gratuitously. (In
>>>> the above commit, it's not clear why the switch was made to using
>>>> openat(): there's no mention of the reasoning in the commit message,
>>>> nor is there anything that is obvious from reading through the code
>>>> change itself.)
>>>
>>> Your code would 'break' if you run with on a new architecture that does
>>> not implement __NR_open, which it is the default for new architecture
>>> on Linux.
>>
>> (Is it the default for new architectures? I was unaware of that.)
>
> Yes, new architectures should use the kernel headers
> include/uapi/asm-generic/unistd.h for syscall definition and the idea
> is to avoid duplication of functionality like syscalls that supplant
> each one (openat and open for instance).  AArch64, for instance, only
> supports on compat more for 32 bits.

Thanks for the info.

>>> In fact I hardly consider this is a 'break' since the user API we
>>> export does not have any constraint which underlying syscall we use.
>>> For instance, a user can seccomp gettimeofday syscall on a system
>>> without vDSO just to found out it is 'broken' on a vDSO kernel.
>>>
>>> I think we should not constraint for this specific usercase; if one
>>> is doing syscall filtering it is expected system level knowledge to
>>> handle all possible syscalls related.  For instance, I would expect
>>> that if the idea is to filtering open() libc implementation the
>>> program should also filter __NR_openat and __NR_openat2 since it
>>> is semantically possible to implement open() with __NR_openat2 if
>>> the syscall is available.
>>
>> Perhaps my use of the word 'break' was a bit too loaded. (And if my
>> mail came across as somehow critical of your patch, that was not my
>> intention, and if you feel it as such, I do apologize for my poor
>> wording.)
>
> In fact I did not take is a criticize, but rather I was worried users
> would expect that kind of syscalls stability over glibc releases.  The
> 'break' word indeed raise a flag ;)

Okay -- I see your philosophical position. I guess my only thought is
that these sort of changes are likely to at least surprise some
people. I'll see whether I can work some hints about this into a
suitable man page.

Cheers,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: Seccomp implications for glibc wrapper function changes

[Michael Kerrisk (man-pages)]

Hi Florian,

On 8 November 2017 at 07:24, Florian Weimer <fweimer@redhat.com> wrote:
> On 11/07/2017 09:35 PM, Michael Kerrisk (man-pages) wrote:
>>
>> This change broke my code that was doing seccomp filtering for the
>> open() system call number (__NR_open). The breakage in question is not
>> serious, since this was really just demonstration code. However, I
>> want to raise awareness that these sorts of changes have the potential
>> to possibly cause breakages for some code using seccomp, and note that
>> I think such changes should not be made lightly or gratuitously.
>
>
> I have the opposite view: We should make such changes as often as possible,
> to remind people that seccomp filters (and certain SELinux and AppArmor
> policies) are incompatible with the GNU/Linux model, where everything is
> developed separately and not maintained within a single source tree (unlike
> say OpenBSD).  This means that you really can't deviate from the upstream
> Linux userspace ABI (in the broadest possible sense) and still expect things
> to work.
>
> I know that people like to slap seccomp filters on everything today, but
> without careful examination, that is likely to introduce bugs (particularly
> on rarely used code paths).  It can also cause the process to switch to
> legacy interfaces with known issues (e.g., reading from /dev/urandom instead
> of getrandom, without waiting for the kernel to signal initialization of the
> pool).

Thanks. The above is a good summary of the counterpoints to my initial argument.

Cheers,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: Seccomp implications for glibc wrapper function changes

[Michael Kerrisk (man-pages)]

[CC += Kees, in case he has some comments]

On 9 November 2017 at 08:17, Michael Kerrisk (man-pages)
<mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Florian,
>
> On 8 November 2017 at 07:24, Florian Weimer <fweimer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> On 11/07/2017 09:35 PM, Michael Kerrisk (man-pages) wrote:
>>>
>>> This change broke my code that was doing seccomp filtering for the
>>> open() system call number (__NR_open). The breakage in question is not
>>> serious, since this was really just demonstration code. However, I
>>> want to raise awareness that these sorts of changes have the potential
>>> to possibly cause breakages for some code using seccomp, and note that
>>> I think such changes should not be made lightly or gratuitously.
>>
>>
>> I have the opposite view: We should make such changes as often as possible,
>> to remind people that seccomp filters (and certain SELinux and AppArmor
>> policies) are incompatible with the GNU/Linux model, where everything is
>> developed separately and not maintained within a single source tree (unlike
>> say OpenBSD).  This means that you really can't deviate from the upstream
>> Linux userspace ABI (in the broadest possible sense) and still expect things
>> to work.
>>
>> I know that people like to slap seccomp filters on everything today, but
>> without careful examination, that is likely to introduce bugs (particularly
>> on rarely used code paths).  It can also cause the process to switch to
>> legacy interfaces with known issues (e.g., reading from /dev/urandom instead
>> of getrandom, without waiting for the kernel to signal initialization of the
>> pool).
>
> Thanks. The above is a good summary of the counterpoints to my initial argument.

Florian, taking your and Adhemerval's useful comments into account, I
added the following text to the seccomp(2) manual page:

[[
   Caveats
       There  are  various  subtleties  to consider when applying seccomp
       filters to a program, including the following:

       *  Some traditional system calls have  user-space  implementations
          in the vdso(7) on many architectures.  Notable examples include
          clock_gettime(2), gettimeofday(2), and time(2).  On such archi‐
          tectures, seccomp filtering for these system calls will have no
          effect.

       *  Seccomp filtering is based on system  call  numbers.   However,
          applications typically do not directly invoke system calls, but
          instead call wrapper functions in the C library which  in  turn
          invoke  the  system  calls.  Consequently, one must be aware of
          the following:

          ·  The glibc wrappers for some  traditional  system  calls  may
             actually  employ  system  calls  with different names in the
             kernel.  For example, the exit(2) wrapper function  actually
             employs the exit_group(2) system call, and the fork(2) wrap‐
             per function actually calls clone(2).

          ·  The behavior of wrapper functions may vary across  architec‐
             tures,  according  to  the range of system calls provided on
             those architectures.  In other words, the same wrapper func‐
             tion  may  invoke different system calls on different archi‐
             tectures.

          ·  Finally, the behavior of wrapper functions can change across
             glibc  versions.   For example, in older versions, the glibc
             wrapper function for open(2) invoked the system call of  the
             same  name,  but  starting in glibc 2.26, the implementation
             switched to calling openat(2) on all architectures.

       The consequence of the above points is that may  be  necessary  to
       filter  for  a  system call other than might be expected.  Various
       manual pages in Section 2 provide helpful details about  the  dif‐
       ferences between wrapper functions and the underlying system calls
       in subsections entitled C library/kernel differences.

       Furthermore, note that the application  of  seccomp  filters  even
       risks causing bugs in an application, when the filters cause unex‐
       pected failures for legitimate  operations  that  the  application
       might  need  to  perform.   Such bugs may not easily be discovered
       when testing the seccomp filters if the bugs occur in rarely  used
       application code paths.
]]

Cheers,

Michael

Re: Seccomp implications for glibc wrapper function changes

[Florian Weimer]

On 11/09/2017 12:58 PM, Michael Kerrisk (man-pages) wrote:
> [CC += Kees, in case he has some comments]
> 
> On 9 November 2017 at 08:17, Michael Kerrisk (man-pages)
> <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Hi Florian,
>>
>> On 8 November 2017 at 07:24, Florian Weimer <fweimer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>>> On 11/07/2017 09:35 PM, Michael Kerrisk (man-pages) wrote:
>>>>
>>>> This change broke my code that was doing seccomp filtering for the
>>>> open() system call number (__NR_open). The breakage in question is not
>>>> serious, since this was really just demonstration code. However, I
>>>> want to raise awareness that these sorts of changes have the potential
>>>> to possibly cause breakages for some code using seccomp, and note that
>>>> I think such changes should not be made lightly or gratuitously.
>>>
>>>
>>> I have the opposite view: We should make such changes as often as possible,
>>> to remind people that seccomp filters (and certain SELinux and AppArmor
>>> policies) are incompatible with the GNU/Linux model, where everything is
>>> developed separately and not maintained within a single source tree (unlike
>>> say OpenBSD).  This means that you really can't deviate from the upstream
>>> Linux userspace ABI (in the broadest possible sense) and still expect things
>>> to work.
>>>
>>> I know that people like to slap seccomp filters on everything today, but
>>> without careful examination, that is likely to introduce bugs (particularly
>>> on rarely used code paths).  It can also cause the process to switch to
>>> legacy interfaces with known issues (e.g., reading from /dev/urandom instead
>>> of getrandom, without waiting for the kernel to signal initialization of the
>>> pool).
>>
>> Thanks. The above is a good summary of the counterpoints to my initial argument.
> 
> Florian, taking your and Adhemerval's useful comments into account, I
> added the following text to the seccomp(2) manual page:
> 
> [[
>     Caveats
>         There  are  various  subtleties  to consider when applying seccomp
>         filters to a program, including the following:
> 
>         *  Some traditional system calls have  user-space  implementations
>            in the vdso(7) on many architectures.  Notable examples include
>            clock_gettime(2), gettimeofday(2), and time(2).  On such archi‐
>            tectures, seccomp filtering for these system calls will have no
>            effect.

I think the situation is more complicated for many of those because they 
can still perform system calls on their fallback paths.  So it's one 
more case where seccomp can give you unpredictable failures.

Rest looks good to me.  Thanks for the writeup.

Florian

Re: Seccomp implications for glibc wrapper function changes

[Adhemerval Zanella]

On 09/11/2017 10:02, Florian Weimer wrote:
> On 11/09/2017 12:58 PM, Michael Kerrisk (man-pages) wrote:
>> [CC += Kees, in case he has some comments]
>>
>> On 9 November 2017 at 08:17, Michael Kerrisk (man-pages)
>> <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> Hi Florian,
>>>
>>> On 8 November 2017 at 07:24, Florian Weimer <fweimer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>>>> On 11/07/2017 09:35 PM, Michael Kerrisk (man-pages) wrote:
>>>>>
>>>>> This change broke my code that was doing seccomp filtering for the
>>>>> open() system call number (__NR_open). The breakage in question is not
>>>>> serious, since this was really just demonstration code. However, I
>>>>> want to raise awareness that these sorts of changes have the potential
>>>>> to possibly cause breakages for some code using seccomp, and note that
>>>>> I think such changes should not be made lightly or gratuitously.
>>>>
>>>>
>>>> I have the opposite view: We should make such changes as often as possible,
>>>> to remind people that seccomp filters (and certain SELinux and AppArmor
>>>> policies) are incompatible with the GNU/Linux model, where everything is
>>>> developed separately and not maintained within a single source tree (unlike
>>>> say OpenBSD).  This means that you really can't deviate from the upstream
>>>> Linux userspace ABI (in the broadest possible sense) and still expect things
>>>> to work.
>>>>
>>>> I know that people like to slap seccomp filters on everything today, but
>>>> without careful examination, that is likely to introduce bugs (particularly
>>>> on rarely used code paths).  It can also cause the process to switch to
>>>> legacy interfaces with known issues (e.g., reading from /dev/urandom instead
>>>> of getrandom, without waiting for the kernel to signal initialization of the
>>>> pool).
>>>
>>> Thanks. The above is a good summary of the counterpoints to my initial argument.
>>
>> Florian, taking your and Adhemerval's useful comments into account, I
>> added the following text to the seccomp(2) manual page:
>>
>> [[
>>     Caveats
>>         There  are  various  subtleties  to consider when applying seccomp
>>         filters to a program, including the following:
>>
>>         *  Some traditional system calls have  user-space  implementations
>>            in the vdso(7) on many architectures.  Notable examples include
>>            clock_gettime(2), gettimeofday(2), and time(2).  On such archi‐
>>            tectures, seccomp filtering for these system calls will have no
>>            effect.
> 
> I think the situation is more complicated for many of those because they can still perform system calls on their fallback paths.  So it's one more case where seccomp can give you unpredictable failures.
> 
> Rest looks good to me.  Thanks for the writeup.
> 
> Florian

Looks good to me as well.

Re: Seccomp implications for glibc wrapper function changes

[Michael Kerrisk (man-pages)]

Hi Florian,

On 9 November 2017 at 13:02, Florian Weimer <fweimer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On 11/09/2017 12:58 PM, Michael Kerrisk (man-pages) wrote:
>>
>> [CC += Kees, in case he has some comments]
>>
>> On 9 November 2017 at 08:17, Michael Kerrisk (man-pages)
>> <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>
>>> Hi Florian,
>>>
>>> On 8 November 2017 at 07:24, Florian Weimer <fweimer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>>>>
>>>> On 11/07/2017 09:35 PM, Michael Kerrisk (man-pages) wrote:
>>>>>
>>>>>
>>>>> This change broke my code that was doing seccomp filtering for the
>>>>> open() system call number (__NR_open). The breakage in question is not
>>>>> serious, since this was really just demonstration code. However, I
>>>>> want to raise awareness that these sorts of changes have the potential
>>>>> to possibly cause breakages for some code using seccomp, and note that
>>>>> I think such changes should not be made lightly or gratuitously.
>>>>
>>>>
>>>>
>>>> I have the opposite view: We should make such changes as often as
>>>> possible,
>>>> to remind people that seccomp filters (and certain SELinux and AppArmor
>>>> policies) are incompatible with the GNU/Linux model, where everything is
>>>> developed separately and not maintained within a single source tree
>>>> (unlike
>>>> say OpenBSD).  This means that you really can't deviate from the
>>>> upstream
>>>> Linux userspace ABI (in the broadest possible sense) and still expect
>>>> things
>>>> to work.
>>>>
>>>> I know that people like to slap seccomp filters on everything today, but
>>>> without careful examination, that is likely to introduce bugs
>>>> (particularly
>>>> on rarely used code paths).  It can also cause the process to switch to
>>>> legacy interfaces with known issues (e.g., reading from /dev/urandom
>>>> instead
>>>> of getrandom, without waiting for the kernel to signal initialization of
>>>> the
>>>> pool).
>>>
>>>
>>> Thanks. The above is a good summary of the counterpoints to my initial
>>> argument.
>>
>>
>> Florian, taking your and Adhemerval's useful comments into account, I
>> added the following text to the seccomp(2) manual page:
>>
>> [[
>>     Caveats
>>         There  are  various  subtleties  to consider when applying seccomp
>>         filters to a program, including the following:
>>
>>         *  Some traditional system calls have  user-space  implementations
>>            in the vdso(7) on many architectures.  Notable examples include
>>            clock_gettime(2), gettimeofday(2), and time(2).  On such archi‐
>>            tectures, seccomp filtering for these system calls will have no
>>            effect.
>
>
> I think the situation is more complicated for many of those because they can
> still perform system calls on their fallback paths.  So it's one more case
> where seccomp can give you unpredictable failures.

Good point. I added the following text:

          (However,  there are cases where the vdso(7) implemen‐
          tations may fall back  to  invoking  the  true  system
          call, in which case seccomp filters would see the sys‐
          tem call.)

> Rest looks good to me.  Thanks for the writeup.

Thanks for the review!

Cheers,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: Seccomp implications for glibc wrapper function changes

[Michael Kerrisk (man-pages)]

[-- Attachment #1: Type: text/plain, Size: 3162 bytes --]

On 9 November 2017 at 13:14, Adhemerval Zanella <
adhemerval.zanella@linaro.org> wrote:
>
>
> On 09/11/2017 10:02, Florian Weimer wrote:
>> On 11/09/2017 12:58 PM, Michael Kerrisk (man-pages) wrote:
>>> [CC += Kees, in case he has some comments]
>>>
>>> On 9 November 2017 at 08:17, Michael Kerrisk (man-pages)
>>> <mtk.manpages@gmail.com> wrote:
>>>> Hi Florian,
>>>>
>>>> On 8 November 2017 at 07:24, Florian Weimer <fweimer@redhat.com> wrote:
>>>>> On 11/07/2017 09:35 PM, Michael Kerrisk (man-pages) wrote:
>>>>>>
>>>>>> This change broke my code that was doing seccomp filtering for the
>>>>>> open() system call number (__NR_open). The breakage in question is
not
>>>>>> serious, since this was really just demonstration code. However, I
>>>>>> want to raise awareness that these sorts of changes have the
potential
>>>>>> to possibly cause breakages for some code using seccomp, and note
that
>>>>>> I think such changes should not be made lightly or gratuitously.
>>>>>
>>>>>
>>>>> I have the opposite view: We should make such changes as often as
possible,
>>>>> to remind people that seccomp filters (and certain SELinux and
AppArmor
>>>>> policies) are incompatible with the GNU/Linux model, where everything
is
>>>>> developed separately and not maintained within a single source tree
(unlike
>>>>> say OpenBSD). This means that you really can't deviate from the
upstream
>>>>> Linux userspace ABI (in the broadest possible sense) and still expect
things
>>>>> to work.
>>>>>
>>>>> I know that people like to slap seccomp filters on everything today,
but
>>>>> without careful examination, that is likely to introduce bugs
(particularly
>>>>> on rarely used code paths). It can also cause the process to switch to
>>>>> legacy interfaces with known issues (e.g., reading from /dev/urandom
instead
>>>>> of getrandom, without waiting for the kernel to signal initialization
of the
>>>>> pool).
>>>>
>>>> Thanks. The above is a good summary of the counterpoints to my initial
argument.
>>>
>>> Florian, taking your and Adhemerval's useful comments into account, I
>>> added the following text to the seccomp(2) manual page:
>>>
>>> [[
>>> Caveats
>>> There are various subtleties to consider when applying seccomp
>>> filters to a program, including the following:
>>>
>>> * Some traditional system calls have user-space implementations
>>> in the vdso(7) on many architectures. Notable examples include
>>> clock_gettime(2), gettimeofday(2), and time(2). On such archi‐
>>> tectures, seccomp filtering for these system calls will have no
>>> effect.
>>
>> I think the situation is more complicated for many of those because they
can still perform system calls on their fallback paths. So it's one more
case where seccomp can give you unpredictable failures.
>>
>> Rest looks good to me. Thanks for the writeup.
>>
>> Florian
>
> Looks good to me as well.

Thanks for the review, Adhemerval!

Cheers,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

[-- Attachment #2: Type: text/html, Size: 4307 bytes --]

Re: Seccomp implications for glibc wrapper function changes

[Kees Cook]

On Thu, Nov 9, 2017 at 5:37 AM, Michael Kerrisk (man-pages)
<mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Florian,
>
> On 9 November 2017 at 13:02, Florian Weimer <fweimer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> On 11/09/2017 12:58 PM, Michael Kerrisk (man-pages) wrote:
>>>
>>> [CC += Kees, in case he has some comments]
>>>
>>> On 9 November 2017 at 08:17, Michael Kerrisk (man-pages)
>>> <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>
>>>> Hi Florian,
>>>>
>>>> On 8 November 2017 at 07:24, Florian Weimer <fweimer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>>>>>
>>>>> On 11/07/2017 09:35 PM, Michael Kerrisk (man-pages) wrote:
>>>>>>
>>>>>>
>>>>>> This change broke my code that was doing seccomp filtering for the
>>>>>> open() system call number (__NR_open). The breakage in question is not
>>>>>> serious, since this was really just demonstration code. However, I
>>>>>> want to raise awareness that these sorts of changes have the potential
>>>>>> to possibly cause breakages for some code using seccomp, and note that
>>>>>> I think such changes should not be made lightly or gratuitously.
>>>>>
>>>>>
>>>>>
>>>>> I have the opposite view: We should make such changes as often as
>>>>> possible,
>>>>> to remind people that seccomp filters (and certain SELinux and AppArmor
>>>>> policies) are incompatible with the GNU/Linux model, where everything is
>>>>> developed separately and not maintained within a single source tree
>>>>> (unlike
>>>>> say OpenBSD).  This means that you really can't deviate from the
>>>>> upstream
>>>>> Linux userspace ABI (in the broadest possible sense) and still expect
>>>>> things
>>>>> to work.
>>>>>
>>>>> I know that people like to slap seccomp filters on everything today, but
>>>>> without careful examination, that is likely to introduce bugs
>>>>> (particularly
>>>>> on rarely used code paths).  It can also cause the process to switch to
>>>>> legacy interfaces with known issues (e.g., reading from /dev/urandom
>>>>> instead
>>>>> of getrandom, without waiting for the kernel to signal initialization of
>>>>> the
>>>>> pool).
>>>>
>>>>
>>>> Thanks. The above is a good summary of the counterpoints to my initial
>>>> argument.
>>>
>>>
>>> Florian, taking your and Adhemerval's useful comments into account, I
>>> added the following text to the seccomp(2) manual page:
>>>
>>> [[
>>>     Caveats
>>>         There  are  various  subtleties  to consider when applying seccomp
>>>         filters to a program, including the following:
>>>
>>>         *  Some traditional system calls have  user-space  implementations
>>>            in the vdso(7) on many architectures.  Notable examples include
>>>            clock_gettime(2), gettimeofday(2), and time(2).  On such archi‐
>>>            tectures, seccomp filtering for these system calls will have no
>>>            effect.
>>
>>
>> I think the situation is more complicated for many of those because they can
>> still perform system calls on their fallback paths.  So it's one more case
>> where seccomp can give you unpredictable failures.
>
> Good point. I added the following text:
>
>           (However,  there are cases where the vdso(7) implemen‐
>           tations may fall back  to  invoking  the  true  system
>           call, in which case seccomp filters would see the sys‐
>           tem call.)
>
>> Rest looks good to me.  Thanks for the writeup.
>
> Thanks for the review!

Agreed, this looks good. Thanks for clarifying it. :)

-Kees

-- 
Kees Cook
Pixel Security

Re: Seccomp implications for glibc wrapper function changes

[Michael Kerrisk (man-pages)]

Hi Kees,

On 13 November 2017 at 23:44, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
[...]
> Agreed, this looks good. Thanks for clarifying it. :)
>
> -Kees

Thanks for checking it.

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/