You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Gilbert Song <so...@gmail.com> on 2019/02/11 19:21:38 UTC
Review Request 69946: Updated handleWhitelistFds() to avoid closing
FDs with FD_CLOEXEC bit.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69946/
-----------------------------------------------------------
Review request for mesos, Andrei Budnik, Jason Lai, Jie Yu, and James Peach.
Repository: mesos
Description
-------
Since this helper is only called right before exec in child process,
and for those open FDs that is set with FD_CLOEXEC flag, they will
be closed during exec, so that we could skip closing these FDs in
the helper. The motivation of this change is to avoid whitelisting
those FDs that have to survive until exec while we do not want to
expose these FDs to user applications.
Diffs
-----
3rdparty/libprocess/src/posix/subprocess.hpp 7bbf91e857538c2a030199a529287efa8ef2c604
Diff: https://reviews.apache.org/r/69946/diff/1/
Testing
-------
Thanks,
Gilbert Song
Re: Review Request 69946: Updated handleWhitelistFds() to avoid
closing FDs with FD_CLOEXEC bit.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69946/#review212706
-----------------------------------------------------------
Fix it, then Ship it!
3rdparty/libprocess/src/posix/subprocess.hpp
Lines 137-138 (patched)
<https://reviews.apache.org/r/69946/#comment298595>
I'd add a TODO here to cleanup the use of os::strerror here as it's not async signal safe. We have the same issue above. So you can follow up with a patch adding TODO.
- Jie Yu
On Feb. 11, 2019, 7:21 p.m., Gilbert Song wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69946/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2019, 7:21 p.m.)
>
>
> Review request for mesos, Andrei Budnik, Jason Lai, Jie Yu, and James Peach.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since this helper is only called right before exec in child process,
> and for those open FDs that is set with FD_CLOEXEC flag, they will
> be closed during exec, so that we could skip closing these FDs in
> the helper. The motivation of this change is to avoid whitelisting
> those FDs that have to survive until exec while we do not want to
> expose these FDs to user applications.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/posix/subprocess.hpp 7bbf91e857538c2a030199a529287efa8ef2c604
>
>
> Diff: https://reviews.apache.org/r/69946/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gilbert Song
>
>
Re: Review Request 69946: Updated handleWhitelistFds() to avoid
closing FDs with FD_CLOEXEC bit.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69946/
-----------------------------------------------------------
(Updated Feb. 11, 2019, 11:33 a.m.)
Review request for mesos, Andrei Budnik, Jason Lai, Jie Yu, and James Peach.
Changes
-------
Addressed comments.
Repository: mesos
Description
-------
Since this helper is only called right before exec in child process,
and for those open FDs that is set with FD_CLOEXEC flag, they will
be closed during exec, so that we could skip closing these FDs in
the helper. The motivation of this change is to avoid whitelisting
those FDs that have to survive until exec while we do not want to
expose these FDs to user applications.
Diffs (updated)
-----
3rdparty/libprocess/src/posix/subprocess.hpp 7bbf91e857538c2a030199a529287efa8ef2c604
Diff: https://reviews.apache.org/r/69946/diff/2/
Changes: https://reviews.apache.org/r/69946/diff/1-2/
Testing
-------
Thanks,
Gilbert Song