[vlc-devel] [PATCH v3 12/14] es_out_timeshift: refactor creation of temporary file

Lyndon Brown jnqnfe at gmail.com
Wed Oct 14 04:56:24 CEST 2020


From: Lyndon Brown <jnqnfe at gmail.com>
Date: Thu, 8 Oct 2020 05:25:00 +0100
Subject: es_out_timeshift: refactor creation of temporary file

...based upon tmpfile() as suggested by Remi.

note, fixes use-after-free that would occur on non-windows should the
fprintf call be re-enabled. (psz_file was free'd in an earlier line).

the loop around the tmpfile() call, as per the comment, is to help get
around any possible 'delete-pending' failure situations on Windows. The
chosen loop count is arbritrary, just copying that of the vlc_mkstemp()
implementation used on Windows.

diff --git a/src/input/es_out_timeshift.c b/src/input/es_out_timeshift.c
index ceb0da2778..ff40988c6a 100644
--- a/src/input/es_out_timeshift.c
+++ b/src/input/es_out_timeshift.c
@@ -39,7 +39,6 @@
 
 #include <vlc_common.h>
 #include <vlc_fs.h>
-#include <vlc_configuration.h>
 #include <vlc_mouse.h>
 #ifdef _WIN32
 #  include <vlc_charset.h>
@@ -195,9 +194,6 @@ struct ts_storage_t
     ts_storage_t *p_next;
 
     /* */
-#ifdef _WIN32
-    char    *psz_file;  /* Filename */
-#endif
     size_t  i_file_max; /* Max size in bytes */
     int64_t i_file_size;/* Current size in bytes */
     FILE    *p_filew;   /* FILE handle for data writing */
@@ -217,7 +213,6 @@ typedef struct
     es_out_t       *p_tsout;
     es_out_t       *p_out;
     int64_t        i_tmp_size_max;
-    const char     *psz_tmp_path;
 
     /* Lock for all following fields */
     vlc_mutex_t    lock;
@@ -257,7 +252,6 @@ typedef struct
 
     /* Configuration */
     int64_t        i_tmp_size_max;    /* Maximal temporary file size in byte */
-    char           *psz_tmp_path;     /* Path for temporary files */
 
     /* Lock for all following fields */
     vlc_mutex_t    lock;
@@ -294,7 +288,7 @@ static int          TsChangeRate( ts_thread_t *, float src_rate, float rate );
 
 static void         *TsRun( void * );
 
-static ts_storage_t *TsStorageNew( const char *psz_path, int64_t i_tmp_size_max );
+static ts_storage_t *TsStorageNew( int64_t i_tmp_size_max );
 static void         TsStorageDelete( ts_storage_t * );
 static void         TsStoragePack( ts_storage_t *p_storage );
 static bool         TsStorageIsFull( ts_storage_t *, const ts_cmd_t *p_cmd );
@@ -322,9 +316,6 @@ static void CmdExecuteDel    ( es_out_t *, ts_cmd_del_t * );
 static int  CmdExecuteControl( es_out_t *, ts_cmd_control_t * );
 static int  CmdExecutePrivControl( es_out_t *, ts_cmd_privcontrol_t * );
 
-/* File helpers */
-static int GetTmpFile( char **path, const char *directory );
-
 static const struct es_out_callbacks es_out_timeshift_cbs;
 
 /*****************************************************************************
@@ -362,37 +353,6 @@ es_out_t *input_EsOutTimeshiftNew( input_thread_t *p_input, es_out_t *p_next_out
     msg_Dbg( p_input, "using timeshift granularity of %d MiB",
              (int)p_sys->i_tmp_size_max/(1024*1024) );
 
-    p_sys->psz_tmp_path = var_InheritString( p_input, "input-timeshift-path" );
-#if defined (_WIN32) && !VLC_WINSTORE_APP
-    if( p_sys->psz_tmp_path == NULL )
-        p_sys->psz_tmp_path = config_GetTempPath();
-    if( p_sys->psz_tmp_path == NULL )
-    {
-        wchar_t *wpath = _wgetcwd( NULL, 0 );
-        if( wpath != NULL )
-        {
-            p_sys->psz_tmp_path = FromWide( wpath );
-            free( wpath );
-        }
-    }
-    if( p_sys->psz_tmp_path == NULL )
-        p_sys->psz_tmp_path = strdup( "C:" );
-
-    if( p_sys->psz_tmp_path != NULL )
-    {
-        size_t len = strlen( p_sys->psz_tmp_path );
-
-        while( len > 0 && p_sys->psz_tmp_path[len - 1] == DIR_SEP_CHAR )
-            len--;
-
-        p_sys->psz_tmp_path[len] = '\0';
-    }
-#endif
-    if( p_sys->psz_tmp_path != NULL )
-        msg_Dbg( p_input, "using timeshift path: %s", p_sys->psz_tmp_path );
-    else
-        msg_Dbg( p_input, "using default timeshift path" );
-
 #if 0
 #define S(t) msg_Err( p_input, "SIZEOF("#t")=%zd", sizeof(t) )
     S(ts_cmd_t);
@@ -424,7 +384,6 @@ static void Destroy( es_out_t *p_out )
         Del( p_out, p_sys->pp_es[0] );
     TAB_CLEAN( p_sys->i_es, p_sys->pp_es  );
 
-    free( p_sys->psz_tmp_path );
     free( p_sys );
 }
 
@@ -879,7 +838,6 @@ static int TsStart( es_out_t *p_out )
         return VLC_EGENERIC;
 
     p_ts->i_tmp_size_max = p_sys->i_tmp_size_max;
-    p_ts->psz_tmp_path = p_sys->psz_tmp_path;
     p_ts->p_input = p_sys->p_input;
     p_ts->p_out = p_sys->p_out;
     p_ts->p_tsout = p_out;
@@ -953,7 +911,7 @@ static void TsPushCmd( ts_thread_t *p_ts, ts_cmd_t *p_cmd )
 
     if( !p_ts->p_storage_w || TsStorageIsFull( p_ts->p_storage_w, p_cmd ) )
     {
-        ts_storage_t *p_storage = TsStorageNew( p_ts->psz_tmp_path, p_ts->i_tmp_size_max );
+        ts_storage_t *p_storage = TsStorageNew( p_ts->i_tmp_size_max );
 
         if( !p_storage )
         {
@@ -1197,42 +1155,44 @@ static const size_t TsStorageSizeofCommand[] =
     [C_PRIVCONTROL] = sizeof(ts_cmd_privcontrol_t)
 };
 
-static ts_storage_t *TsStorageNew( const char *psz_tmp_path, int64_t i_tmp_size_max )
+static ts_storage_t *TsStorageNew( int64_t i_tmp_size_max )
 {
     ts_storage_t *p_storage = malloc( sizeof (*p_storage) );
     if( unlikely(p_storage == NULL) )
         return NULL;
 
-    char *psz_file;
-    int fd = GetTmpFile( &psz_file, psz_tmp_path );
-    if( fd == -1 )
+    /* Using loop to help avoid possible delete-pending failure situations on Windows */
+    FILE *p_file;
+    for (unsigned i = 0; i < 256; i++)
     {
-        free( p_storage );
-        return NULL;
+        p_file = tmpfile();
+        if (p_file != NULL)
+            break;
     }
+    if( unlikely(p_file == NULL) )
+        goto error;
+
+    int fd_r = vlc_dup(fileno(p_file));
+    if( unlikely(fd_r == -1) )
+        goto error;
 
-    p_storage->p_filew = fdopen( fd, "w+b" );
+    p_storage->p_filew = freopen( NULL, "w+b", p_file ); // Change to write only
     if( p_storage->p_filew == NULL )
     {
-        vlc_close( fd );
-        vlc_unlink( psz_file );
+        fclose( p_file );
+        vlc_close( fd_r );
         goto error;
     }
+    p_file = NULL;
 
-    p_storage->p_filer = vlc_fopen( psz_file, "rb" );
+    p_storage->p_filer = fdopen( fd_r, "rb" );
     if( p_storage->p_filer == NULL )
     {
         fclose( p_storage->p_filew );
-        vlc_unlink( psz_file );
+        vlc_close( fd_r );
         goto error;
     }
 
-#ifndef _WIN32
-    vlc_unlink( psz_file );
-    free( psz_file );
-#else
-    p_storage->psz_file = psz_file;
-#endif
     p_storage->p_next = NULL;
 
     /* */
@@ -1244,7 +1204,7 @@ static ts_storage_t *TsStorageNew( const char *psz_tmp_path, int64_t i_tmp_size_
     p_storage->i_cmd_buf = TS_STORAGE_COMMAND_PREALLOC * MAX_COMMAND_SIZE;
     p_storage->p_cmd_w = p_storage->p_cmd_buf;
     p_storage->p_cmd_r = p_storage->p_cmd_buf;
-    //fprintf( stderr, "\nSTORAGE name=%s size=%d KiB\n", p_storage->psz_file, p_storage->i_cmd_max * sizeof(*p_storage->p_cmd) /1024 );
+    //fprintf( stderr, "\nSTORAGE name=%s size=%d KiB\n", p_storage->i_cmd_max * sizeof(*p_storage->p_cmd) /1024 );
 
     if( !p_storage->p_cmd_buf )
     {
@@ -1253,7 +1213,6 @@ static ts_storage_t *TsStorageNew( const char *psz_tmp_path, int64_t i_tmp_size_
     }
     return p_storage;
 error:
-    free( psz_file );
     free( p_storage );
     return NULL;
 }
@@ -1272,10 +1231,6 @@ static void TsStorageDelete( ts_storage_t *p_storage )
 
     fclose( p_storage->p_filer );
     fclose( p_storage->p_filew );
-#ifdef _WIN32
-    vlc_unlink( p_storage->psz_file );
-    free( p_storage->psz_file );
-#endif
     free( p_storage );
 }
 
@@ -1807,22 +1762,3 @@ static int CmdExecutePrivControl( es_out_t *p_tsout, ts_cmd_privcontrol_t *p_cmd
     default: vlc_assert_unreachable();
     }
 }
-
-static int GetTmpFile( char **path, const char *directory )
-{
-    int fd = -1;
-    if (directory == NULL) {
-        char *tmpdir = config_GetTempPath();
-        fd = GetTmpFile(path, tmpdir);
-        free(tmpdir);
-        return fd;
-    }
-    vlc_mkdir(directory, 0700);
-
-    if (asprintf(path, "%s" DIR_SEP PACKAGE_NAME "-timeshift.XXXXXX", directory) >= 0) {
-        fd = vlc_mkstemp(*path);
-        if (fd == -1)
-            free(*path);
-    }
-    return fd;
-}



More information about the vlc-devel mailing list