mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 50477: Fixed use-after-close bug when using libevent and SSL.
Date Thu, 28 Jul 2016 23:31:56 GMT


> On July 27, 2016, 8:31 p.m., Joseph Wu wrote:
> > LGTM + some comment tweaks.
> > 
> > This review is almost a revert of this patch: https://github.com/apache/mesos/commit/ca3667f4e97e11ad30811753fdb52bc02854113f
> > You may want to reference that commit and/or the reasoning behind it.

Will do, thanks!


> On July 27, 2016, 8:31 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/src/libevent_ssl_socket.cpp, lines 144-146
> > <https://reviews.apache.org/r/50477/diff/1/?file=1454471#file1454471line144>
> >
> >     This comment is no longer accurate.

Good eye!


> On July 27, 2016, 8:31 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/src/libevent_ssl_socket.cpp, lines 106-110
> > <https://reviews.apache.org/r/50477/diff/1/?file=1454471#file1454471line106>
> >
> >     This comment still seems relevant.

Yeah, it's still relevant. However, I removed this comment because it seemed like a very lengthy
way of saying: don't access 'this' after the 'this' is destructed. When I was reading this
code and trying to understand all of it, this was one comment that didn't help (it seemed
very clear to me from the code). Are you ok with removing it?


- Benjamin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50477/#review143784
-----------------------------------------------------------


On July 27, 2016, 2:03 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50477/
> -----------------------------------------------------------
> 
> (Updated July 27, 2016, 2:03 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Joseph Wu.
> 
> 
> Bugs: MESOS-5913
>     https://issues.apache.org/jira/browse/MESOS-5913
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The deferred call to SSL_shutdown within ~LibeventSSLSocketImpl
> can occur after the socket fd has been closed by Socket::~Impl.
> 
> This can lead to a TLS Alert message being sent on any fd if
> it the fd is re-used between the close and the SSL_shutdown!
> 
> Thanks to Jan-Philip Gehrcke for reporting the issue.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/socket.hpp 881b44b987e5894cac838dae046ab7dbad20b000

>   3rdparty/libprocess/src/libevent_ssl_socket.hpp 4d376d8b7c1b29105de69bed2e4077f8c94fed0b

>   3rdparty/libprocess/src/libevent_ssl_socket.cpp f4c0b0b97960400b0282837979bf0ed17f56a068

> 
> Diff: https://reviews.apache.org/r/50477/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Ran my repro steps (issue HTTP requests while hammering the master with HTTPS requests).
Issue disappears after this fix.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message