PSI: multiple free

Gauthier Lamaison <[email protected]>
Newsgroups gmane.comp.video.videolan.libdvbpsi.devel
Message-ID <[email protected]>
Hi,

I found some potentials multiple frees in the function Gather*Sections 
of each PSI tables.

When a table is complete the process is to save the current information, 
chain the sections, decode the sections, delete the sections (by calling 
dvbpsi_DeletePSISections), signal the new table, and reinitialize the 
structures.

The problem is that dvpsi_ReInit* will call  dvbpsi_ReInitDecoder which 
will delete any none null sections.

The multiple free will only occur if you have multiple sections in your 
table because when Gather*Sections calls dvbpsi_DeletePSISections it 
will reset the first section (p_*_decoder->ap_sections[0] = NULL;) but 
it will not reset the other sections which may have been freed by 
calling dvbpsi_DeletePSISections.

Here is a example with the NIT table, let's say we have two sections for 
the nit (with section number 0 et and section number 1).

When the table is completed, we'll call dvbpsi_ChainSectionsDecoder 
which will link p_nit_decoder->ap_sections[0] and 
p_nit_decoder->ap_sections[1].

Then we'll call dvbpsi_DeletePSISections ( 
dvbpsi_DeletePSISections(p_nit_decoder->ap_sections[0]) ) and free the 
two sections (because p_nit_decoder->ap_sections[0] and 
p_nit_decoder->ap_sections[1] are linked). But we'll only reset the 
first section (p_nit_decoder->ap_sections[0] = NULL) which means that 
the second section will point to a freed memory area.

But when we called dvpsi_ReInitNIT, dvbpsi_DeletePSISections will be 
called again on each section:

  for (unsigned int i = 0; i <= 255; i++)
{
     if (p_decoder->ap_sections[i])
         dvbpsi_DeletePSISections(p_decoder->ap_sections[i]);
     p_decoder->ap_sections[i] = NULL;
}

There is no problem when i == 0 because we had reset that pointer but 
when i == 1 the pointer is not null but points to a freed memory area 
which will cause the double free.

But the main problem is that dvbpsi_DeletePSISections will delete the 
section passed as a parameter and all the sections linked to that 
section (and after dvbpsi_ChainSectionsDecoder all the section are 
linked as a single linked list) and when dvpsi_ReInitNIT is called, it 
will call dvbpsi_DeletePSISections on each non null section so if we 
have 3 section we'll get:

dvbpsi_DeletePSISections(p_decoder->ap_sections[0]) : free(sections[0]) 
-> free(sections[1]) -> free(sections[2])
dvbpsi_DeletePSISections(p_decoder->ap_sections[1]) : free(sections[1]) 
-> free(sections[2])
dvbpsi_DeletePSISections(p_decoder->ap_sections[2]) : free(sections[2])

Here is what I propose:

First, remove the direct call to dvbpsi_DeletePSISections from 
Gather*Sections, then add a boolean to dvbpsi_ReInitDecoder which will 
indicate if the sections are chained or not so we'll know if we need to 
call dvbpsi_DeletePSISections for each section or if we just need to 
reset the pointers in the loop.

I will send you the different patch  if my solution is right for you.

Regards,
Gauthier

_______________________________________________
libdvbpsi-devel mailing list
[email protected]
http://mailman.videolan.org/listinfo/libdvbpsi-devel
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.