[vlc-devel] [PATCH] check NULL pointers

[Lucas C. Villa Real]

Laurent Aimar <fenrir <at> babylon.via.ecp.fr> writes:
> 
> Hi,
> 
> On Tue, Mar 30, 2010 at 02:08:37PM -0300, Lucas C. Villa Real wrote:
> > While working with the GIT snapshot I realized that some transport
> > streams carrying high profile H.264 streams were causing VLC to crash.
> > A quick investigation pointed the cause to be NULL pointer exceptions.
> > Please consider merging the patch below.
>  Could you share a sample ?

Sure. I truncated them to 15MB each. They are available for download here:
http://lucasvr.gobolinux.org/etc/h264-crash.ts
http://lucasvr.gobolinux.org/etc/h264-crash-2.ts

>  I don't see how it is possible for EsOutProgramGetMetaName to be called
> with a NULL p_pgrm. If it is, then it means there is a bug elsewhere that
> should be addressed instead of being workarounded here.

Sorry, this should not have been included in the patch. I introduced this while
trying to find where the NULL pointer was coming from.

>  Same here, EsOutProgramMeta cannot be called with a NULL p_meta.

Ditto here, I apologize. p_meta was indeed valid when the function was called,
and the following is the contents from that structure when the crash happens:

(gdb) print *p_meta
$2 = {ppsz_meta = {0x87e5f10, "\016Globo 1Seg", 0x0 <repeats 16 times>},
extra_tags = {i_size = 0, p_entries = 0x0}, i_status = 0}

So, given that extra_tags is empty, vlc_meta_CopyExtraNames() doesn't return 
any valid entries and thus the call to *ppsz_all_keys[0] causes the crash.

> ppsz_all_keys cannot be NULL, vlc_meta_CopyExtraNames will segfault
> if no more memory...
>  The test *ppsz_all_keys (probably more consitant if ppsz_all_keys[0] were
> used) seems to fix a real bug to me. Could you resend that in a patch ?

Yes, sure. Please find it below. I can confirm that the above is enough to fix
the crash.

> free(NULL) is accepted in VLC, so it is not needed.

Alright. Thanks for reviewing this.

Lucas


Fixes a crash in VLC caused by a missing verification for a NULL pointer.
Signed-off-by: Lucas C. Villa Real <lucasvr at lsi.usp.br>

diff --git a/src/input/es_out.c b/src/input/es_out.c
index bbcbcd7..8d348b1 100644
--- a/src/input/es_out.c
+++ b/src/input/es_out.c
@@ -1234,7 +1234,7 @@ static void EsOutProgramMeta( es_out_t *out, int i_group,
const vlc_meta_t *p_me
     char **ppsz_all_keys = vlc_meta_CopyExtraNames(p_meta );

     info_category_t *p_cat = NULL;
-    if( psz_provider || *ppsz_all_keys[0] )
+    if( psz_provider || ( ppsz_all_keys[0] && *ppsz_all_keys[0] ) )
     {
         char *psz_cat = EsOutProgramGetMetaName( p_pgrm );
         if( psz_cat )