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 Mahler <bm...@apache.org> on 2017/09/02 00:38:03 UTC
Review Request 62049: Fixed an OOM due to a send loop for SSL sockets.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62049/
-----------------------------------------------------------
Review request for mesos, Benno Evers, Chun-Hung Hsiao, Gilbert Song, Greg Mann, Jie Yu, and Joseph Wu.
Bugs: MESOS-7934
https://issues.apache.org/jira/browse/MESOS-7934
Repository: mesos
Description
-------
Per MESOS-7934, the LibeventSSLSocket incorrectly returns 0 to the
sender when an EOF or "dirty" SSL shutdown occurs (i.e. TCP close
before SSL close). Not only is this incorrect due to the caller
re-sending the same data again. In some cases (one is reproduced
in a follow-up test), this can trigger a loop in libprocess that
rapidly leads to an OOM.
The fix here is to fail the send instead. This is the best we can
do with libevent 2.0.x since it does not set BEV_EVENT_READING or
BEV_EVENT_WRITING for SSL buffevents. With libevent 2.1.x, we can
fix our logic to deal with the read and write side events separately.
https://github.com/libevent/libevent/commit/f7eb69ace
Comments are added in a follow up change to explain this for
posterity, and MESOS-7930 tracks the additional tech debt that
needs to be addressed for SSL socket support.
Diffs
-----
3rdparty/libprocess/src/libevent_ssl_socket.cpp 0fa7565d1dc49e53890708df154c4d8ea278ad37
Diff: https://reviews.apache.org/r/62049/diff/1/
Testing
-------
Tested in subsequent patch.
Thanks,
Benjamin Mahler
Re: Review Request 62049: Fixed an OOM due to a send loop for SSL
sockets.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62049/#review184410
-----------------------------------------------------------
Ship it!
Ship It!
- Jie Yu
On Sept. 2, 2017, 12:38 a.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62049/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2017, 12:38 a.m.)
>
>
> Review request for mesos, Benno Evers, Chun-Hung Hsiao, Gilbert Song, Greg Mann, Jie Yu, and Joseph Wu.
>
>
> Bugs: MESOS-7934
> https://issues.apache.org/jira/browse/MESOS-7934
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Per MESOS-7934, the LibeventSSLSocket incorrectly returns 0 to the
> sender when an EOF or "dirty" SSL shutdown occurs (i.e. TCP close
> before SSL close). Not only is this incorrect due to the caller
> re-sending the same data again. In some cases (one is reproduced
> in a follow-up test), this can trigger a loop in libprocess that
> rapidly leads to an OOM.
>
> The fix here is to fail the send instead. This is the best we can
> do with libevent 2.0.x since it does not set BEV_EVENT_READING or
> BEV_EVENT_WRITING for SSL buffevents. With libevent 2.1.x, we can
> fix our logic to deal with the read and write side events separately.
>
> https://github.com/libevent/libevent/commit/f7eb69ace
>
> Comments are added in a follow up change to explain this for
> posterity, and MESOS-7930 tracks the additional tech debt that
> needs to be addressed for SSL socket support.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/libevent_ssl_socket.cpp 0fa7565d1dc49e53890708df154c4d8ea278ad37
>
>
> Diff: https://reviews.apache.org/r/62049/diff/1/
>
>
> Testing
> -------
>
> Tested in subsequent patch.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 62049: Fixed an OOM due to a send loop for SSL
sockets.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62049/
-----------------------------------------------------------
(Updated Sept. 2, 2017, 3:10 a.m.)
Review request for mesos, Benno Evers, Chun-Hung Hsiao, Gilbert Song, Greg Mann, Jie Yu, and Joseph Wu.
Changes
-------
Updated description to more accurately describe the issue per jie's analysis.
Bugs: MESOS-7934
https://issues.apache.org/jira/browse/MESOS-7934
Repository: mesos
Description (updated)
-------
Per MESOS-7934, the LibeventSSLSocket incorrectly returns 0 to the
sender when an EOF, or "dirty" SSL shutdown (i.e. TCP close before
SSL close), or a send is performed on a socket after it has been
shut down. Not only is this incorrect due to the caller re-sending
the same data again, in the case that the socket has been shut down,
the caller of send will enter an infinite loop of retrying the send
which will rapidly lead to an OOM in libprocess.
The fix here is to fail the send instead. Note that with libevent
2.0.x the 'events' will not contain BEV_EVENT_READING or
BEV_EVENT_WRITING for SSL buffevents. With libevent 2.1.x, we can
update our logic to deal with the read and write side events
separately.
https://github.com/libevent/libevent/commit/f7eb69ace
Comments are added in a follow up change to explain this for
posterity, and MESOS-7930 tracks the additional tech debt that
needs to be addressed for SSL socket support.
Diffs (updated)
-----
3rdparty/libprocess/src/libevent_ssl_socket.cpp 0fa7565d1dc49e53890708df154c4d8ea278ad37
Diff: https://reviews.apache.org/r/62049/diff/2/
Changes: https://reviews.apache.org/r/62049/diff/1-2/
Testing
-------
Tested in subsequent patch.
Thanks,
Benjamin Mahler
Re: Review Request 62049: Fixed an OOM due to a send loop for SSL
sockets.
Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62049/#review184407
-----------------------------------------------------------
Ship it!
Ship It!
- Chun-Hung Hsiao
On Sept. 2, 2017, 12:38 a.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62049/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2017, 12:38 a.m.)
>
>
> Review request for mesos, Benno Evers, Chun-Hung Hsiao, Gilbert Song, Greg Mann, Jie Yu, and Joseph Wu.
>
>
> Bugs: MESOS-7934
> https://issues.apache.org/jira/browse/MESOS-7934
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Per MESOS-7934, the LibeventSSLSocket incorrectly returns 0 to the
> sender when an EOF or "dirty" SSL shutdown occurs (i.e. TCP close
> before SSL close). Not only is this incorrect due to the caller
> re-sending the same data again. In some cases (one is reproduced
> in a follow-up test), this can trigger a loop in libprocess that
> rapidly leads to an OOM.
>
> The fix here is to fail the send instead. This is the best we can
> do with libevent 2.0.x since it does not set BEV_EVENT_READING or
> BEV_EVENT_WRITING for SSL buffevents. With libevent 2.1.x, we can
> fix our logic to deal with the read and write side events separately.
>
> https://github.com/libevent/libevent/commit/f7eb69ace
>
> Comments are added in a follow up change to explain this for
> posterity, and MESOS-7930 tracks the additional tech debt that
> needs to be addressed for SSL socket support.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/libevent_ssl_socket.cpp 0fa7565d1dc49e53890708df154c4d8ea278ad37
>
>
> Diff: https://reviews.apache.org/r/62049/diff/1/
>
>
> Testing
> -------
>
> Tested in subsequent patch.
>
>
> Thanks,
>
> Benjamin Mahler
>
>