From: Steinar H. Gunderson <sgunderson@bigfoot.com>
Date: Wed, 28 Jun 2017 17:50:32 +0000 (+0200)
Subject: Fix a use-after-free when using video inputs. Found by GCC's -fsanitize=address.
X-Git-Tag: 1.6.1~24
X-Git-Url: https://git.sesse.net/?p=nageru;a=commitdiff_plain;h=fa0b850bb90894ae9686e0ad7a17ed1b2aafb5d1

Fix a use-after-free when using video inputs. Found by GCC's -fsanitize=address.
---

diff --git a/mixer.cpp b/mixer.cpp
index 3ce49c1..31277f1 100644
--- a/mixer.cpp
+++ b/mixer.cpp
@@ -1334,7 +1334,7 @@ void Mixer::render_one_frame(int64_t duration)
 	// The theme can't (or at least shouldn't!) call connect_signal() on
 	// each FFmpeg input, so we'll do it here.
 	for (const pair<LiveInputWrapper *, FFmpegCapture *> &conn : theme->get_signal_connections()) {
-		conn.first->connect_signal_raw(conn.second->get_card_index());
+		conn.first->connect_signal_raw(conn.second->get_card_index(), input_state);
 	}
 
 	// If HDMI/SDI output is active and the user has requested auto mode,
diff --git a/theme.cpp b/theme.cpp
index 22c940a..ed34fae 100644
--- a/theme.cpp
+++ b/theme.cpp
@@ -775,12 +775,12 @@ void LiveInputWrapper::connect_signal(int signal_num)
 	}
 
 	signal_num = theme->map_signal(signal_num);
-	connect_signal_raw(signal_num);
+	connect_signal_raw(signal_num, *theme->input_state);
 }
 
-void LiveInputWrapper::connect_signal_raw(int signal_num)
+void LiveInputWrapper::connect_signal_raw(int signal_num, const InputState &input_state)
 {
-	BufferedFrame first_frame = theme->input_state->buffered_frames[signal_num][0];
+	BufferedFrame first_frame = input_state.buffered_frames[signal_num][0];
 	if (first_frame.frame == nullptr) {
 		// No data yet.
 		return;
@@ -792,10 +792,10 @@ void LiveInputWrapper::connect_signal_raw(int signal_num)
 		height = userdata->last_height[first_frame.field_number];
 	}
 
-	movit::YCbCrLumaCoefficients ycbcr_coefficients = theme->input_state->ycbcr_coefficients[signal_num];
-	bool full_range = theme->input_state->full_range[signal_num];
+	movit::YCbCrLumaCoefficients ycbcr_coefficients = input_state.ycbcr_coefficients[signal_num];
+	bool full_range = input_state.full_range[signal_num];
 
-	if (theme->input_state->ycbcr_coefficients_auto[signal_num]) {
+	if (input_state.ycbcr_coefficients_auto[signal_num]) {
 		full_range = false;
 
 		// The Blackmagic driver docs claim that the device outputs Y'CbCr
@@ -819,7 +819,7 @@ void LiveInputWrapper::connect_signal_raw(int signal_num)
 
 	BufferedFrame last_good_frame = first_frame;
 	for (unsigned i = 0; i < max(ycbcr_inputs.size(), rgba_inputs.size()); ++i) {
-		BufferedFrame frame = theme->input_state->buffered_frames[signal_num][i];
+		BufferedFrame frame = input_state.buffered_frames[signal_num][i];
 		if (frame.frame == nullptr) {
 			// Not enough data; reuse last frame (well, field).
 			// This is suboptimal, but we have nothing better.
@@ -871,7 +871,7 @@ void LiveInputWrapper::connect_signal_raw(int signal_num)
 	}
 
 	if (deinterlace) {
-		BufferedFrame frame = theme->input_state->buffered_frames[signal_num][0];
+		BufferedFrame frame = input_state.buffered_frames[signal_num][0];
 		CHECK(deinterlace_effect->set_int("current_field_position", frame.field_number));
 	}
 }
@@ -1027,6 +1027,7 @@ Theme::Chain Theme::get_chain(unsigned num, float t, unsigned width, unsigned he
 	chain.setup_chain = [this, funcref, input_state]{
 		unique_lock<mutex> lock(m);
 
+		assert(this->input_state == nullptr);
 		this->input_state = &input_state;
 
 		// Set up state, including connecting signals.
@@ -1036,6 +1037,8 @@ Theme::Chain Theme::get_chain(unsigned num, float t, unsigned width, unsigned he
 			exit(1);
 		}
 		assert(lua_gettop(L) == 0);
+
+		this->input_state = nullptr;
 	};
 
 	// TODO: Can we do better, e.g. by running setup_chain() and seeing what it references?
diff --git a/theme.h b/theme.h
index 9e9672e..e1c3ed1 100644
--- a/theme.h
+++ b/theme.h
@@ -84,7 +84,7 @@ private:
 
 	std::mutex m;
 	lua_State *L;  // Protected by <m>.
-	const InputState *input_state;  // Protected by <m>. Only set temporarily, during chain setup.
+	const InputState *input_state = nullptr;  // Protected by <m>. Only set temporarily, during chain setup.
 	movit::ResourcePool *resource_pool;
 	int num_channels;
 	unsigned num_cards;
@@ -108,8 +108,8 @@ public:
 	// Note: <override_bounce> is irrelevant for PixelFormat_8BitBGRA.
 	LiveInputWrapper(Theme *theme, movit::EffectChain *chain, bmusb::PixelFormat pixel_format, bool override_bounce, bool deinterlace);
 
-	void connect_signal(int signal_num);
-	void connect_signal_raw(int signal_num);
+	void connect_signal(int signal_num);  // Must be called with the theme's <m> lock held, since it accesses theme->input_state.
+	void connect_signal_raw(int signal_num, const InputState &input_state);
 	movit::Effect *get_effect() const
 	{
 		if (deinterlace) {