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 <bb...@apache.org> on 2019/11/21 21:42:11 UTC
Review Request 71781: Used a potential use after free bug.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71781/
-----------------------------------------------------------
Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
Repository: mesos
Description
-------
Moving a framework to `frameworks.completed` transfers ownership of the
Framework to that container and e.g., using the original framework
pointer after that could fail (the framework could e.g., be already be
rotated out).
This patch fixes the master to not use the potentially cleaned up
framework pointer anymore.
Diffs
-----
src/master/master.cpp 14b90a5e276df055bb8a570331f27cab200c9869
Diff: https://reviews.apache.org/r/71781/diff/1/
Testing
-------
`make check`
Thanks,
Benjamin Bannier
Re: Review Request 71781: Used a potential use after free bug.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71781/#review218744
-----------------------------------------------------------
Ship it!
Ideally we would have a regression test, but I won't push on that :P
src/master/master.cpp
Lines 11483-11489 (original), 11483-11491 (patched)
<https://reviews.apache.org/r/71781/#comment306618>
Looks good, I guess we could also just send the event before moving into the completed set to avoid the copy and the cognitive overhead of the extra variable?
- Benjamin Mahler
On Nov. 21, 2019, 9:42 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71781/
> -----------------------------------------------------------
>
> (Updated Nov. 21, 2019, 9:42 p.m.)
>
>
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Moving a framework to `frameworks.completed` transfers ownership of the
> Framework to that container and e.g., using the original framework
> pointer after that could fail (the framework could e.g., be already be
> rotated out).
>
> This patch fixes the master to not use the potentially cleaned up
> framework pointer anymore.
>
>
> Diffs
> -----
>
> src/master/master.cpp 14b90a5e276df055bb8a570331f27cab200c9869
>
>
> Diff: https://reviews.apache.org/r/71781/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71781: Used a potential use after free bug.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71781/#review218749
-----------------------------------------------------------
Ship it!
Looks good, but the title line seems to have a typo? I hope you're *not* using that use-after-free bug for anything. :D
- Benno Evers
On Nov. 21, 2019, 9:42 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71781/
> -----------------------------------------------------------
>
> (Updated Nov. 21, 2019, 9:42 p.m.)
>
>
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Moving a framework to `frameworks.completed` transfers ownership of the
> Framework to that container and e.g., using the original framework
> pointer after that could fail (the framework could e.g., be already be
> rotated out).
>
> This patch fixes the master to not use the potentially cleaned up
> framework pointer anymore.
>
>
> Diffs
> -----
>
> src/master/master.cpp 14b90a5e276df055bb8a570331f27cab200c9869
>
>
> Diff: https://reviews.apache.org/r/71781/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>