You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <be...@mesosphere.io> on 2017/06/12 09:31:08 UTC
Review Request 59995: Used dedicated invocation of getsid to get the
current process' session.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59995/
-----------------------------------------------------------
Review request for mesos and Benjamin Mahler.
Repository: mesos
Description
-------
To get the session of the current process 'getsid(pid_t)' provides
special when passed a zero argument instead of a process ID. This
patch uses that special invocation instead of calling
'getsid(getpid())'.
For some versions of the 'getsid' syscall as implemented e.g., for the
Linux kernel 'getsid(0)' will directly return a session ID while
passing any non-zero process ID while invoke code to to check
whether the calling code is permitted to get the session ID of the
given process. This also calls into e.g., SELinux. Certain SELinux
rule implementations can fail to detect that the given process ID is
the current one, and thus fail the 'getsid' invocation with 'EPERM'.
Calling 'getsid' with a zero process ID prevents calling into security
hooks at all.
Diffs
-----
3rdparty/stout/tests/os/process_tests.cpp d1760ef3cc36c1c13f1ab00ea4ab17fda4a46d8b
Diff: https://reviews.apache.org/r/59995/diff/1/
Testing
-------
Tested on internal CI on various configurations.
Thanks,
Benjamin Bannier
Re: Review Request 59995: Used dedicated invocation of getsid to get
the current process' session.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59995/#review177646
-----------------------------------------------------------
Ship it!
3rdparty/stout/tests/os/process_tests.cpp
Line 82 (original), 82 (patched)
<https://reviews.apache.org/r/59995/#comment251334>
Interesting that this works (getting the session id from /proc) but the getsid call doesn't.
3rdparty/stout/tests/os/process_tests.cpp
Line 86 (original), 86 (patched)
<https://reviews.apache.org/r/59995/#comment251335>
I was thinking we might want to document this, but since `getsid(0)` seems more idiomatic for getting your own session id, doesn't seem necessary.
- Benjamin Mahler
On June 12, 2017, 9:31 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59995/
> -----------------------------------------------------------
>
> (Updated June 12, 2017, 9:31 a.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> To get the session of the current process 'getsid(pid_t)' provides
> special when passed a zero argument instead of a process ID. This
> patch uses that special invocation instead of calling
> 'getsid(getpid())'.
>
> For some versions of the 'getsid' syscall as implemented e.g., for the
> Linux kernel 'getsid(0)' will directly return a session ID while
> passing any non-zero process ID while invoke code to to check
> whether the calling code is permitted to get the session ID of the
> given process. This also calls into e.g., SELinux. Certain SELinux
> rule implementations can fail to detect that the given process ID is
> the current one, and thus fail the 'getsid' invocation with 'EPERM'.
> Calling 'getsid' with a zero process ID prevents calling into security
> hooks at all.
>
>
> Diffs
> -----
>
> 3rdparty/stout/tests/os/process_tests.cpp d1760ef3cc36c1c13f1ab00ea4ab17fda4a46d8b
>
>
> Diff: https://reviews.apache.org/r/59995/diff/1/
>
>
> Testing
> -------
>
> Tested on internal CI on various configurations.
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 59995: Used dedicated invocation of getsid to get
the current process' session.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59995/#review177600
-----------------------------------------------------------
Patch looks great!
Reviews applied: [59995]
Passed command: support\windows-build.bat
- Mesos Reviewbot Windows
On June 12, 2017, 9:31 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59995/
> -----------------------------------------------------------
>
> (Updated June 12, 2017, 9:31 a.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> To get the session of the current process 'getsid(pid_t)' provides
> special when passed a zero argument instead of a process ID. This
> patch uses that special invocation instead of calling
> 'getsid(getpid())'.
>
> For some versions of the 'getsid' syscall as implemented e.g., for the
> Linux kernel 'getsid(0)' will directly return a session ID while
> passing any non-zero process ID while invoke code to to check
> whether the calling code is permitted to get the session ID of the
> given process. This also calls into e.g., SELinux. Certain SELinux
> rule implementations can fail to detect that the given process ID is
> the current one, and thus fail the 'getsid' invocation with 'EPERM'.
> Calling 'getsid' with a zero process ID prevents calling into security
> hooks at all.
>
>
> Diffs
> -----
>
> 3rdparty/stout/tests/os/process_tests.cpp d1760ef3cc36c1c13f1ab00ea4ab17fda4a46d8b
>
>
> Diff: https://reviews.apache.org/r/59995/diff/1/
>
>
> Testing
> -------
>
> Tested on internal CI on various configurations.
>
>
> Thanks,
>
> Benjamin Bannier
>
>