[vlc-commits] [Git][videolan/vlc][master] 4 commits: demux: fix invalid free

Hugo Beauzée-Luyssen (@chouquette) gitlab at videolan.org
Mon Jan 10 07:21:17 UTC 2022



Hugo Beauzée-Luyssen pushed to branch master at VideoLAN / VLC


Commits:
0b17f567 by Lyndon Brown at 2022-01-10T07:03:38+00:00
demux: fix invalid free

upon `asprintf()` failure, the pointer variable used to capture
the result is left in an undefined state.

here, should such failure occur, the later `free(modbuf)` call
would be invalid. we must reset this variable to null upon
failure to fix this.

note that `modbuf` is not the same variable as `module`, which
seems to be causing some confusion in review. this patch has
absolutely no impact upon the operation of this function other
than fixing the bug just described only. it **does not** cause
the module search to operate upon a null search string. it does
not change the search string at all.

note that `module` initially holds a search string owned by the
caller of the function. the function may though need to replace
this with a new string that it must allocate on the heap. for
the purposes of ensuring that the function later only releases
the new string, a copy of the allocated string pointer is kept
in the `modbuf` variable. in fact the string pointer in `module`
is only replaced with the newly allocated one upon success of
`asprintf()`. thus, should the allocation fail, `module` will
remain as `"any"` (the allocation only occurs under an `"any"`
based search). the patch thus does not alter the search string,
and thus does not impact module loading in any way.

- - - - -
d81e2005 by Lyndon Brown at 2022-01-10T07:03:38+00:00
demux: abort module loading upon search string creation failure

if in "any" mode and falling back to an extension based match, and if the
`asprintf()` call fails to create the extension based shortcut search
string, the consensus is that it should give up rather than proceed to try
an "any" based demux module load.

note that the `modbuf = NULL` statement was just resetting the undefined
state of the variable from the failed `asprintf()` call to ensure that the
later `free(modbuf)` was valid. it is not needed now that we go down the
error path.

note, the init of `s->psz_filepath` in `vlc_stream_CustomNew()` is not
strictly necessary to avoid `free()` on an initialised attribute, since
`calloc()` is actually involved in the object creation - much of the
initialisation done in `vlc_stream_CustomNew()` is actually redundant.

- - - - -
830ea36e by Lyndon Brown at 2022-01-10T07:03:38+00:00
stream: remove pointless & incomplete initialisation

`vlc_custom_create()` uses `calloc()` to allocate the memory, so
initialising all of the attributes individually to null/0/false is
unnecessary.

the initialisation was not even complete, missing all of these:
 - s->psz_name
 - s->psz_location
 - s->b_preparsing
 - s->out
 - s->pf_demux

- - - - -
b10f5a02 by Lyndon Brown at 2022-01-10T07:03:38+00:00
core: remove pointless re-initialisation of `stream_t` attributes

- - - - -


4 changed files:

- src/input/access.c
- src/input/demux.c
- src/input/stream.c
- src/input/stream_fifo.c


Changes:

=====================================
src/input/access.c
=====================================
@@ -103,9 +103,7 @@ static stream_t *access_New(vlc_object_t *parent, input_thread_t *input,
 
     access->p_input_item = input ? input_GetItem(input) : NULL;
     access->out = out;
-    access->psz_name = NULL;
     access->psz_url = strdup(mrl);
-    access->psz_filepath = NULL;
     access->b_preparsing = preparsing;
     priv = vlc_stream_Private(access);
 


=====================================
src/input/demux.c
=====================================
@@ -156,11 +156,6 @@ demux_t *demux_NewAdvanced( vlc_object_t *p_obj, input_thread_t *p_input,
     p_demux->out            = out;
     p_demux->b_preparsing   = b_preparsing;
 
-    p_demux->pf_readdir = NULL;
-    p_demux->pf_demux   = NULL;
-    p_demux->pf_control = NULL;
-    p_demux->p_sys      = NULL;
-
     char *modbuf = NULL;
     bool strict = true;
 
@@ -185,6 +180,8 @@ demux_t *demux_NewAdvanced( vlc_object_t *p_obj, input_thread_t *p_input,
             else
             if (likely(asprintf(&modbuf, "ext-%s", ext + 1) >= 0))
                 module = modbuf;
+            else
+                goto error;
         }
         strict = false;
     }
@@ -194,13 +191,11 @@ demux_t *demux_NewAdvanced( vlc_object_t *p_obj, input_thread_t *p_input,
     free(modbuf);
 
     if (priv->module == NULL)
-    {
-        free( p_demux->psz_filepath );
         goto error;
-    }
 
     return p_demux;
 error:
+    free( p_demux->psz_filepath );
     free( p_demux->psz_name );
     stream_CommonDelete( p_demux );
     return NULL;
@@ -454,16 +449,9 @@ static demux_t *demux_FilterNew( demux_t *p_next, const char *p_name )
     if (unlikely(p_demux == NULL))
         return NULL;
 
-    priv = vlc_stream_Private(p_demux);
-    p_demux->s            = p_next;
-    p_demux->p_input_item = NULL;
-    p_demux->p_sys        = NULL;
-    p_demux->psz_name     = NULL;
-    p_demux->psz_url      = NULL;
-    p_demux->psz_location = NULL;
-    p_demux->psz_filepath = NULL;
-    p_demux->out          = NULL;
+    p_demux->s = p_next;
 
+    priv = vlc_stream_Private(p_demux);
     priv->module = module_need(p_demux, "demux_filter", p_name,
                                p_name != NULL);
     if (priv->module == NULL)


=====================================
src/input/stream.c
=====================================
@@ -74,30 +74,14 @@ stream_t *vlc_stream_CustomNew(vlc_object_t *parent,
     if (unlikely(priv == NULL))
         return NULL;
 
-    stream_t *s = &priv->stream;
-
-    s->psz_url = NULL;
-    s->s = NULL;
-    s->pf_read = NULL;
-    s->pf_block = NULL;
-    s->pf_readdir = NULL;
-    s->pf_seek = NULL;
-    s->pf_control = NULL;
-    s->p_sys = NULL;
-    s->p_input_item = NULL;
     assert(destroy != NULL);
     priv->destroy = destroy;
-    priv->block = NULL;
-    priv->peek = NULL;
-    priv->offset = 0;
-    priv->eof = false;
 
     /* UTF16 and UTF32 text file conversion */
     priv->text.conv = (vlc_iconv_t)(-1);
     priv->text.char_width = 1;
-    priv->text.little_endian = false;
 
-    return s;
+    return &priv->stream;
 }
 
 void *vlc_stream_Private(stream_t *stream)


=====================================
src/input/stream_fifo.c
=====================================
@@ -123,7 +123,6 @@ vlc_stream_fifo_t *vlc_stream_fifo_New(vlc_object_t *parent, stream_t **reader)
     sys = vlc_stream_Private(s);
     sys->writer = writer;
     s->pf_block = vlc_stream_fifo_Block;
-    s->pf_seek = NULL;
     s->pf_control = vlc_stream_fifo_Control;
     *reader = s;
     return writer;



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/147a1bd812ab3d95aaeda127775a068bfdbc88b0...b10f5a02e1787ce044226094e257ff3449e4c8d8

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




More information about the vlc-commits mailing list