[vlc-commits] [Git][videolan/vlc][master] 6 commits: visual: fix potential use-after-free

Steve Lhomme (@robUx4) gitlab at videolan.org
Fri Nov 4 14:01:54 UTC 2022



Steve Lhomme pushed to branch master at VideoLAN / VLC


Commits:
d1b78570 by Alexandre Janniaux at 2022-11-04T13:27:07+00:00
visual: fix potential use-after-free

p_outpic is used to modify b_progressive before it is checked to be
non-null.

- - - - -
540677ae by Alexandre Janniaux at 2022-11-04T13:27:07+00:00
visualization: visual: use internal picture pool

Remove the dependency to vout_GetPicture by creating an internal picture
pool inside the visualization module. For now the number of picture is
arbitrary, since nothing in the design allows passing the number of
required additional pictures, but it's no different than before.

- - - - -
4d71ef49 by Alexandre Janniaux at 2022-11-04T13:27:07+00:00
visualization: goom: ensure b_progressive=true

With the next change into a picture_pool, we must ensure we don't create
interlaced pictures from here.

- - - - -
467949c9 by Alexandre Janniaux at 2022-11-04T13:27:07+00:00
visualization: goom: use internal picture pool

Remove the dependency to vout_GetPicture by creating an internal picture
pool inside the visualization module. For now the number of picture is
arbitrary, since nothing in the design allows passing the number of
required additional pictures, but it's no different than before.

- - - - -
af1278e2 by Alexandre Janniaux at 2022-11-04T13:27:07+00:00
video_output: remove obsolete constraint

Decoder has been pushing pictures from their own pool since the
introduction of the push model.

- - - - -
2345ecc7 by Alexandre Janniaux at 2022-11-04T13:27:07+00:00
video_output: remove vout_GetPicture

The function was only used by the two last visualization modules, but is
now unused and unfit to the purpose of push model. Pools should be
created locally instead of from the video output.

Removing the function allows simplifying the video output.

- - - - -


5 changed files:

- include/vlc_vout.h
- modules/visualization/goom.c
- modules/visualization/visual/visual.c
- src/libvlccore.sym
- src/video_output/video_output.c


Changes:

=====================================
include/vlc_vout.h
=====================================
@@ -119,7 +119,6 @@ VLC_API int vout_GetSnapshot( vout_thread_t *p_vout,
                               const char *psz_format, vlc_tick_t i_timeout );
 
 /* */
-VLC_API picture_t * vout_GetPicture( vout_thread_t * );
 VLC_API void vout_PutPicture( vout_thread_t *, picture_t * );
 
 /* Subpictures channels ID */


=====================================
modules/visualization/goom.c
=====================================
@@ -34,6 +34,7 @@
 #include <vlc_aout.h>            /* aout_FormatNbChannels, AOUT_FMTS_SIMILAR */
 #include <vlc_vout.h>              /* vout_*Picture, aout_filter_..tVout */
 #include <vlc_filter.h>
+#include <vlc_picture_pool.h>
 
 #include <goom/goom.h>
 
@@ -81,6 +82,7 @@ typedef struct
     video_format_t fmt;
 
     vout_thread_t *p_vout;
+    picture_pool_t *pool;
     int           i_speed;
 
     vlc_mutex_t   lock;
@@ -128,10 +130,19 @@ static int Open( vlc_object_t *p_this )
     p_thread->fmt.i_chroma = VLC_CODEC_RGB32;
     p_thread->fmt.i_sar_num = p_thread->fmt.i_sar_den = 1;
 
+    /* TODO: the number of picture is arbitrary for now. */
+    p_thread->pool = picture_pool_NewFromFormat(&p_thread->fmt, 3);
+    if (p_thread->pool == NULL)
+    {
+        free(p_thread);
+        return VLC_ENOMEM;
+    }
+
     p_thread->p_vout = aout_filter_GetVout( p_filter, &p_thread->fmt );
     if( p_thread->p_vout == NULL )
     {
         msg_Err( p_filter, "no suitable vout module" );
+        picture_pool_Release(p_thread->pool);
         free( p_thread );
         return VLC_EGENERIC;
     }
@@ -152,6 +163,7 @@ static int Open( vlc_object_t *p_this )
     {
         msg_Err( p_filter, "cannot launch goom thread" );
         vout_Close( p_thread->p_vout );
+        picture_pool_Release(p_thread->pool);
         free( p_thread );
         return VLC_EGENERIC;
     }
@@ -322,7 +334,7 @@ static void *Thread( void *p_thread_data )
         plane = goom_update( p_plugin_info, p_data, 0, 0.0,
                              NULL, NULL );
 
-        p_pic = vout_GetPicture( p_thread->p_vout );
+        p_pic = picture_pool_Wait( p_thread->pool );
         if( unlikely(p_pic == NULL) )
             continue;
 
@@ -330,6 +342,7 @@ static void *Thread( void *p_thread_data )
                 p_thread->fmt.i_width * p_thread->fmt.i_height * 4 );
 
         p_pic->date = date_Get( &i_pts ) + GOOM_DELAY;
+        p_pic->b_progressive = true;
         vout_PutPicture( p_thread->p_vout, p_pic );
     }
 
@@ -359,6 +372,7 @@ static void Close( filter_t *p_filter )
         block_Release( p_thread->pp_blocks[p_thread->i_blocks] );
     }
 
+    picture_pool_Release(p_thread->pool);
     free( p_thread );
 }
 


=====================================
modules/visualization/visual/visual.c
=====================================
@@ -36,6 +36,7 @@
 #include <vlc_aout.h>
 #include <vlc_filter.h>
 #include <vlc_queue.h>
+#include <vlc_picture_pool.h>
 
 #include "visual.h"
 
@@ -177,6 +178,7 @@ typedef struct
 {
     vlc_queue_t     queue;
     vout_thread_t   *p_vout;
+    picture_pool_t  *pool;
     visual_effect_t **effect;
     int             i_effect;
     bool            dead;
@@ -202,6 +204,7 @@ static int Open( vlc_object_t *p_this )
     p_sys = p_filter->p_sys = malloc( sizeof( filter_sys_t ) );
     if( unlikely (p_sys == NULL ) )
         return VLC_EGENERIC;
+    p_sys->pool = NULL;
 
     int width = var_InheritInteger( p_filter , "effect-width");
     int height = var_InheritInteger( p_filter , "effect-height");
@@ -305,6 +308,12 @@ static int Open( vlc_object_t *p_this )
         .primaries = COLOR_PRIMARIES_SRGB,
         .space = COLOR_SPACE_SRGB,
     };
+
+    /* TODO: the number of picture is arbitrary for now. */
+    p_sys->pool = picture_pool_NewFromFormat(&fmt, 3);
+    if (p_sys->pool == NULL)
+        goto error;
+
     p_sys->p_vout = aout_filter_GetVout( p_filter, &fmt );
     if( p_sys->p_vout == NULL )
     {
@@ -327,6 +336,9 @@ static int Open( vlc_object_t *p_this )
     return VLC_SUCCESS;
 
 error:
+    if (p_sys->pool)
+        picture_pool_Release(p_sys->pool);
+
     for( int i = 0; i < p_sys->i_effect; i++ )
         free( p_sys->effect[i] );
     free( p_sys->effect );
@@ -339,10 +351,10 @@ static block_t *DoRealWork( filter_t *p_filter, block_t *p_in_buf )
     filter_sys_t *p_sys = p_filter->p_sys;
 
     /* First, get a new picture */
-    picture_t *p_outpic = vout_GetPicture( p_sys->p_vout );
-    p_outpic->b_progressive = true;
+    picture_t *p_outpic = picture_pool_Wait(p_sys->pool);
     if( unlikely(p_outpic == NULL) )
         return p_in_buf;
+    p_outpic->b_progressive = true;
 
     /* Blank the picture */
     for( int i = 0 ; i < p_outpic->i_planes ; i++ )
@@ -416,6 +428,7 @@ static void Close( filter_t * p_filter )
 #undef p_effect
     }
 
+    picture_pool_Release(p_sys->pool);
     free( p_sys->effect );
     free( p_sys );
 }


=====================================
src/libvlccore.sym
=====================================
@@ -739,7 +739,6 @@ vlm_New
 vout_Close
 vout_Hold
 vout_Release
-vout_GetPicture
 vout_PutPicture
 vout_PutSubpicture
 vout_RegisterSubpictureChannel


=====================================
src/video_output/video_output.c
=====================================
@@ -378,32 +378,9 @@ void vout_SetSpuHighlight( vout_thread_t *vout,
         spu_SetHighlight(sys->spu, spu_hl);
 }
 
-/**
- * Allocates a video output picture buffer.
- *
- * Either vout_PutPicture() or picture_Release() must be used to return the
- * buffer to the video output free buffer pool.
- *
- * You may use picture_Hold() (paired with picture_Release()) to keep a
- * read-only reference.
- */
-picture_t *vout_GetPicture(vout_thread_t *vout)
-{
-    vout_thread_sys_t *sys = VOUT_THREAD_TO_SYS(vout);
-    assert(!sys->dummy);
-    picture_t *picture = picture_pool_Wait(sys->private.display_pool);
-    if (likely(picture != NULL)) {
-        picture_Reset(picture);
-        video_format_CopyCropAr(&picture->format, &sys->original);
-    }
-    return picture;
-}
-
 /**
  * It gives to the vout a picture to be displayed.
  *
- * The given picture MUST comes from vout_GetPicture.
- *
  * Becareful, after vout_PutPicture is called, picture_t::p_next cannot be
  * read/used.
  */



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/c30e3123d0c3bac31f6dca06bb49fd50dfa7e07d...2345ecc7ac8fd61ea8d18775b89bbc4ea75c08e9

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/c30e3123d0c3bac31f6dca06bb49fd50dfa7e07d...2345ecc7ac8fd61ea8d18775b89bbc4ea75c08e9
You're receiving this email because of your account on code.videolan.org.


VideoLAN code repository instance


More information about the vlc-commits mailing list