Re: Seccomp implications for glibc wrapper function changes

["Michael Kerrisk (man-pages)" <mtk dot manpages at gmail dot com>]

Hello Adhemerval

On 8 November 2017 at 02:18, Adhemerval Zanella
<adhemerval.zanella@linaro.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@linaro.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@linaro.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/