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

diff --git a/mixer.cpp b/mixer.cpp
index 3ce49c18cf9bda311a17604d025607b647c8310c..31277f1cd87af1bf2e03a67a1f169dcdf8f8c03a 100644 (file)
--- 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 22c940aeb594fced3e409862b3e020df24648a0b..ed34faed2c9a1411ce7417c06da9ab68f5e1fd40 100644 (file)
--- 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 9e9672e0fa5b98d23b8edd897a587a84cc9360ca..e1c3ed1baa0baa0820705a6c4428ea8d95d9c2d0 100644 (file)
--- 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) {