generic-library/webrtc
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Prevent potential double-free if srtp_create fails.

Browse Source 
If srtp_create fails while adding streams, it deallocates the session
but doesn't clear the passed pointer which then could lead to a
double-free in the SrtpSession dtor.

The CL also adds locking for libsrtp initialization / shutdown.

BUG=4042
R=jiayl@webrtc.org, juberti@google.com, pthatcher@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/47319004

Cr-Commit-Position: refs/heads/master@{#9300}
This commit is contained in:
Joachim Bauch 2015-05-27 23:41:43 +02:00
parent 10602605f8
commit fec2c6d7eb
4 changed files with 27 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
talk/session/media/srtpfilter.cc
View File

@ -474,6 +474,7 @@ bool SrtpFilter::ParseKeyParams(const std::string& key_params,
#ifdef HAVE_SRTP #ifdef HAVE_SRTP
bool SrtpSession::inited_ = false; bool SrtpSession::inited_ = false;
rtc::GlobalLockPod SrtpSession::lock_;
SrtpSession::SrtpSession() SrtpSession::SrtpSession()
: session_(NULL), : session_(NULL),
@ -691,6 +692,7 @@ bool SrtpSession::SetKey(int type, const std::string& cs,
int err = srtp_create(&session_, &policy); int err = srtp_create(&session_, &policy);
if (err != err_status_ok) { if (err != err_status_ok) {
session_ = NULL;
LOG(LS_ERROR) << "Failed to create SRTP session, err=" << err; LOG(LS_ERROR) << "Failed to create SRTP session, err=" << err;
return false; return false;
} }
@ -702,6 +704,8 @@ bool SrtpSession::SetKey(int type, const std::string& cs,
} }
bool SrtpSession::Init() { bool SrtpSession::Init() {
rtc::GlobalLockScope ls(&lock_);
if (!inited_) { if (!inited_) {
int err; int err;
err = srtp_init(); err = srtp_init();
@ -729,6 +733,8 @@ bool SrtpSession::Init() {
} }
void SrtpSession::Terminate() { void SrtpSession::Terminate() {
rtc::GlobalLockScope ls(&lock_);
if (inited_) { if (inited_) {
int err = srtp_shutdown(); int err = srtp_shutdown();
if (err) { if (err) {

2
talk/session/media/srtpfilter.h
View File

@ -36,6 +36,7 @@
#include "talk/media/base/cryptoparams.h" #include "talk/media/base/cryptoparams.h"
#include "webrtc/p2p/base/sessiondescription.h" #include "webrtc/p2p/base/sessiondescription.h"
#include "webrtc/base/basictypes.h" #include "webrtc/base/basictypes.h"
#include "webrtc/base/criticalsection.h"
#include "webrtc/base/scoped_ptr.h" #include "webrtc/base/scoped_ptr.h"
#include "webrtc/base/sigslotrepeater.h" #include "webrtc/base/sigslotrepeater.h"
@ -242,6 +243,7 @@ class SrtpSession {
int rtcp_auth_tag_len_; int rtcp_auth_tag_len_;
rtc::scoped_ptr<SrtpStat> srtp_stat_; rtc::scoped_ptr<SrtpStat> srtp_stat_;
static bool inited_; static bool inited_;
static rtc::GlobalLockPod lock_;
int last_send_seq_num_; int last_send_seq_num_;
DISALLOW_COPY_AND_ASSIGN(SrtpSession); DISALLOW_COPY_AND_ASSIGN(SrtpSession);
}; };

9
webrtc/base/criticalsection.cc
View File

@ -152,4 +152,13 @@ GlobalLock::GlobalLock() {
lock_acquired = 0; lock_acquired = 0;
} }
GlobalLockScope::GlobalLockScope(GlobalLockPod* lock)
: lock_(lock) {
lock_->Lock();
}
GlobalLockScope::~GlobalLockScope() {
lock_->Unlock();
}
} // namespace rtc } // namespace rtc

10
webrtc/base/criticalsection.h
View File

@ -114,6 +114,16 @@ class GlobalLock : public GlobalLockPod {
GlobalLock(); GlobalLock();
}; };
// GlobalLockScope, for serializing execution through a scope.
class SCOPED_LOCKABLE GlobalLockScope {
public:
explicit GlobalLockScope(GlobalLockPod* lock) EXCLUSIVE_LOCK_FUNCTION(lock);
~GlobalLockScope() UNLOCK_FUNCTION();
private:
GlobalLockPod* const lock_;
DISALLOW_COPY_AND_ASSIGN(GlobalLockScope);
};
} // namespace rtc } // namespace rtc
#endif // WEBRTC_BASE_CRITICALSECTION_H_ #endif // WEBRTC_BASE_CRITICALSECTION_H_