From ff538e4adf99a47320657a93af7df8da33f029f8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= <sebastian@centricular.com>
Date: Fri, 23 Aug 2024 15:56:41 +0300
Subject: [PATCH] clock: Fix unchecked overflows in linear regression code

The initial calculation for the precision shift was wrong and would allow for
overflows during the calculations which were not detected and lead to wrong
results.

Also add a test for a case where overflows where previously not detected and
caused a completely wrong result.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/7431>
---
 subprojects/gstreamer/gst/gstutils.c          | 18 +++++++--
 .../gstreamer/tests/check/gst/gstutils.c      | 39 ++++++++++++++++++-
 2 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/subprojects/gstreamer/gst/gstutils.c b/subprojects/gstreamer/gst/gstutils.c
index 24880d3691..9cce0480a2 100644
--- a/subprojects/gstreamer/gst/gstutils.c
+++ b/subprojects/gstreamer/gst/gstutils.c
@@ -4638,10 +4638,20 @@ gst_calculate_linear_regression (const GstClockTime * xy,
    */
 
   /* Guess how many bits we might need for the usual distribution of input,
-   * with a fallback loop that drops precision if things go pear-shaped */
-  max_bits = gst_log2 (MAX (xmax - xmin, ymax - ymin)) * 7 / 8 + gst_log2 (n);
-  if (max_bits > 64)
-    pshift = max_bits - 64;
+   * with a fallback loop that drops precision if things go pear-shaped.
+   *
+   * Each calculation of tmp during the iteration is multiplying two numbers and
+   * then adding them together, or adding them together and then multiplying
+   * them. The second case is worse and means we need at least twice
+   * (multiplication) as many bits as the biggest number needs, plus another bit
+   * (addition). At most 63 bits (signed 64 bit integer) are available.
+   *
+   * That means that each number must require at most (63 - 1) / 2 bits = 31
+   * bits of storage.
+   */
+  max_bits = gst_log2 (MAX (1, MAX (xmax - xmin, ymax - ymin)));
+  if (max_bits > 31)
+    pshift = max_bits - 31;
 
   i = 0;
   do {
diff --git a/subprojects/gstreamer/tests/check/gst/gstutils.c b/subprojects/gstreamer/tests/check/gst/gstutils.c
index 0697c8405f..ab76f19d36 100644
--- a/subprojects/gstreamer/tests/check/gst/gstutils.c
+++ b/subprojects/gstreamer/tests/check/gst/gstutils.c
@@ -1894,6 +1894,41 @@ static const GstClockTime times4[] = {
   60, 100
 };
 
+static const GstClockTime times5[] = {
+  3097492530135419, 3097492507369715,
+  3097492729101770, 3097492707369715,
+  3097493128807704, 3097493107369715,
+  3097493328386576, 3097493307369715,
+  3097493728105969, 3097493707369715,
+  3097494207585880, 3097494187369715,
+  3097495048016903, 3097495027369715,
+  3097496287625601, 3097496267369715,
+  3097497447848684, 3097497427369715,
+  3097498888090268, 3097498867369715,
+  3097499128283739, 3097499107369715,
+  3097501168351161, 3097501147369715,
+  3097502448473187, 3097502427369715,
+  3097502729226046, 3097502707369715,
+  3097503009343256, 3097502987369715,
+  3097503089910694, 3097503067369715,
+  3097503131675320, 3097503107369715,
+  3097503179587096, 3097503147369715,
+  3097504741040566, 3097504707369715,
+  3097504941855342, 3097504907369715,
+  3097506820823108, 3097506787369715,
+  3097508259718567, 3097508227369715,
+  3097508421611551, 3097508387369715,
+  3097509422069495, 3097509387369715,
+  3097511460842652, 3097511427369715,
+  3097511740612276, 3097511707369715,
+  3097513169836240, 3097513147369715,
+  3097514020101841, 3097513987369715,
+  3097515540180269, 3097515507369715,
+  3097516618966664, 3097516587369715,
+  3097518098778648, 3097518067369715,
+  3097518938513564, 3097518907369715
+};
+
 struct test_entry
 {
   gint n;
@@ -1912,7 +1947,9 @@ struct test_entry
       2321009753483354451}, {
         32, times3, 291922315691409, 162234934150296, 1370930728180888261,
       4392719527011673456}, {
-      6, times4, 60, 100, 2, 1}
+      6, times4, 60, 100, 2, 1},
+  {G_N_ELEMENTS (times5) / 2, times5, 3097518938513564, 3097518903890035,
+      7927902279407500000, 7932215807091104881},
 };
 
 GST_START_TEST (test_regression)