[FFmpeg-devel] [PATCH 4/5] avfilter/vf_morpho: Fix invalid frees on error

[Andreas Rheinhardt]

The current code used a pointer to an array (of arrays) that
is offset relative to the start of the actually allocated buffer.
Yet offsetting the pointer is only done on success, whereas the
freeing code believes it to have happened even on error.
So if any of the subarrays (or the subarrays' subarrays) can't
be successfully allocated, one gets a bad free in free_lut().

Furthermore, said offsetting is only permissible in case the
offsetted pointer points in the allocated buffer (here: in case
the LUT's min_r is <= 0), as pointer arithmetic is undefined
in case it exceeds the allocated object.

Moreover, in case one of the subarrays couldn't be allocated,
the code nevertheless tried to free the subarray's subarrays;
and in case one of the subarray's subarrays could not be allocated
successfully, there will be an invalid free, too, because the
pointers for the subarrays' subarrays are also offset compared
to the base pointer.

This commit fixes all of this, by using the actually allocated
pointer for freeing and by adding appropriate checks before
freeing the subarrays. The former also allows to distinguish
the cases in which the lut is currently only half-allocated due to
an error in an earlier allocation attempt from the success case.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
---
 libavfilter/vf_morpho.c | 49 +++++++++++++++++++++++------------------
 1 file changed, 28 insertions(+), 21 deletions(-)

diff --git a/libavfilter/vf_morpho.c b/libavfilter/vf_morpho.c
index 8c1e084e7e..818ebd6b9a 100644
--- a/libavfilter/vf_morpho.c
+++ b/libavfilter/vf_morpho.c
@@ -61,7 +61,10 @@ typedef struct IPlane {
 } IPlane;
 
 typedef struct LUT {
+    /* arr is shifted from base_arr by FFMAX(min_r, 0).
+     * arr != NULL means "lut completely allocated" */
     uint8_t ***arr;
+    uint8_t ***base_arr;
     int min_r;
     int max_r;
     int I;
@@ -262,7 +265,8 @@ static void maxinplace16_fun(uint8_t *aa, const uint8_t *bb, int x)
 
 static int alloc_lut(LUT *Ty, chord_set *SE, int type_size, int mode)
 {
-    const int size = Ty->max_r + 1 - Ty->min_r;
+    const int min = FFMAX(Ty->min_r, 0);
+    const int max = min + (Ty->max_r - Ty->min_r);
     int pre_pad_x = 0;
 
     if (SE->minX < 0)
@@ -270,55 +274,58 @@ static int alloc_lut(LUT *Ty, chord_set *SE, int type_size, int mode)
     Ty->pre_pad_x = pre_pad_x;
     Ty->type_size = type_size;
 
-    Ty->arr = av_calloc(size, sizeof(*Ty->arr));
-    if (!Ty->arr)
+    Ty->base_arr = av_calloc(max + 1, sizeof(*Ty->base_arr));
+    if (!Ty->base_arr)
         return AVERROR(ENOMEM);
-    for (int r = 0; r < Ty->max_r - Ty->min_r + 1; r++) {
-        Ty->arr[r] = av_calloc(Ty->I, sizeof(uint8_t *));
-        if (!Ty->arr[r])
+    for (int r = min; r <= max; r++) {
+        uint8_t **arr = Ty->base_arr[r] = av_calloc(Ty->I, sizeof(uint8_t *));
+        if (!Ty->base_arr[r])
             return AVERROR(ENOMEM);
         for (int i = 0; i < Ty->I; i++) {
-            Ty->arr[r][i] = av_calloc(Ty->X + pre_pad_x, type_size);
-            if (!Ty->arr[r][i])
+            arr[i] = av_calloc(Ty->X + pre_pad_x, type_size);
+            if (!arr[i])
                 return AVERROR(ENOMEM);
             if (mode == ERODE)
-                memset(Ty->arr[r][i], UINT8_MAX, pre_pad_x * type_size);
+                memset(arr[i], UINT8_MAX, pre_pad_x * type_size);
             /* Shifting the X index such that negative indices correspond to
              * the pre-padding.
              */
-            Ty->arr[r][i] = &(Ty->arr[r][i][pre_pad_x * type_size]);
+            arr[i] = &(arr[i][pre_pad_x * type_size]);
         }
     }
 
-    Ty->arr = &(Ty->arr[0 - Ty->min_r]);
+    Ty->arr = &(Ty->base_arr[min - Ty->min_r]);
 
     return 0;
 }
 
 static void free_lut(LUT *table)
 {
-    uint8_t ***rp;
+    const int min = FFMAX(table->min_r, 0);
+    const int max = min + (table->max_r - table->min_r);
 
-    if (!table->arr)
+    if (!table->base_arr)
         return;
 
-    // The R index was shifted, create a pointer to the original array
-    rp = &(table->arr[table->min_r]);
-
-    for (int r = table->min_r; r <= table->max_r; r++) {
+    for (int r = min; r <= max; r++) {
+        if (!table->base_arr[r])
+            break;
         for (int i = 0; i < table->I; i++) {
+            if (!table->base_arr[r][i])
+                break;
             // The X index was also shifted, for padding purposes.
-            av_free(table->arr[r][i] - table->pre_pad_x * table->type_size);
+            av_free(table->base_arr[r][i] - table->pre_pad_x * table->type_size);
         }
-        av_freep(&table->arr[r]);
+        av_freep(&table->base_arr[r]);
     }
-    av_freep(&rp);
+    av_freep(&table->base_arr);
+    table->arr = NULL;
 }
 
 static int alloc_lut_if_necessary(LUT *Ty, IPlane *f, chord_set *SE,
                                   int y, int num, enum MorphModes mode)
 {
-    if (Ty->I != SE->Lnum ||
+    if (!Ty->arr || Ty->I != SE->Lnum ||
         Ty->X != f->w ||
         SE->minX < 0 && -SE->minX > Ty->pre_pad_x ||
         Ty->min_r != SE->minY ||
-- 
2.30.2