From 5caeab5f6d12fe0fc69e708a829cfdbec3a401b4 Mon Sep 17 00:00:00 2001
From: Romain Vimont <rom@rom1v.com>
Date: Sat, 26 Jun 2021 15:53:05 +0200
Subject: [PATCH] Fix v4l2 data race

The v4l2_sink implementation directly read the internal video_buffer
field "pending_frame_consumed", which is protected by the internal
video_buffer mutex. But this mutex was not locked, so reads were racy.

Lock using the v4l2_sink mutex in addition, and use a separate field to
avoid depending on the video_buffer internal data.
---
 app/src/v4l2_sink.c | 13 ++++++++++---
 app/src/v4l2_sink.h |  1 +
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/app/src/v4l2_sink.c b/app/src/v4l2_sink.c
index a276b79c..1f6c264c 100644
--- a/app/src/v4l2_sink.c
+++ b/app/src/v4l2_sink.c
@@ -112,7 +112,7 @@ run_v4l2_sink(void *data) {
     for (;;) {
         sc_mutex_lock(&vs->mutex);
 
-        while (!vs->stopped && vs->vb.pending_frame_consumed) {
+        while (!vs->stopped && !vs->has_frame) {
             sc_cond_wait(&vs->cond, &vs->mutex);
         }
 
@@ -121,9 +121,11 @@ run_v4l2_sink(void *data) {
             break;
         }
 
+        video_buffer_consume(&vs->vb, vs->frame);
+        vs->has_frame = false;
+
         sc_mutex_unlock(&vs->mutex);
 
-        video_buffer_consume(&vs->vb, vs->frame);
         bool ok = encode_and_write_frame(vs, vs->frame);
         av_frame_unref(vs->frame);
         if (!ok) {
@@ -241,6 +243,7 @@ sc_v4l2_sink_open(struct sc_v4l2_sink *vs) {
         goto error_av_frame_free;
     }
 
+    vs->has_frame = false;
     vs->header_written = false;
     vs->stopped = false;
 
@@ -299,14 +302,18 @@ sc_v4l2_sink_close(struct sc_v4l2_sink *vs) {
 
 static bool
 sc_v4l2_sink_push(struct sc_v4l2_sink *vs, const AVFrame *frame) {
+    sc_mutex_lock(&vs->mutex);
+
     bool ok = video_buffer_push(&vs->vb, frame, NULL);
     if (!ok) {
         return false;
     }
 
-    // signal possible change of vs->vb.pending_frame_consumed
+    vs->has_frame = true;
     sc_cond_signal(&vs->cond);
 
+    sc_mutex_unlock(&vs->mutex);
+
     return true;
 }
 
diff --git a/app/src/v4l2_sink.h b/app/src/v4l2_sink.h
index 81bcdd1e..aad25926 100644
--- a/app/src/v4l2_sink.h
+++ b/app/src/v4l2_sink.h
@@ -22,6 +22,7 @@ struct sc_v4l2_sink {
     sc_thread thread;
     sc_mutex mutex;
     sc_cond cond;
+    bool has_frame;
     bool stopped;
     bool header_written;
 
-- 
GitLab