[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