[libdvbpsi-devel] PSI: multiple free

[Gauthier Lamaison]

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