Hello, On July 11th, Morten Sjøgren reported a segfault due to the double free of a buffer in ifoFree_PTL_MAIT and provided a patch: http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/2010-July/066025.html The patch was taken into account by Dominik 'Rathann' Mierzejewski on September 2nd: http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/2010-September/066033.ht... The patch is incomplete as ifoRead_PTL_MAIT can still return without resetting the pointer in ifofile->ptl_mait. In my case, I have two DVDs failing due an invalid seek offset passed to DVDFileSeek_(). The following patch set ifofile->ptl_mait to NULL (not 0 as in the original patch) before every return statement. Moreover, if the seek offset is obviously out of range, the loop over the countries is interrupted and the nr_of_countries is set to the index of the highest country that could be read. I'm not sure it is the proper action but without a test case to disprove it, it sound sensible as nr_of_countries doesn't seem to be used. Frederic
Hello,
- free(ptl_mait->countries[i].pf_ptl_mai); + if (ptl_mait->countries[i].pf_ptl_mai) + free(ptl_mait->countries[i].pf_ptl_mai); AFAIK free(NULL) is totally valid so this change is just a non-op.
Best regards -- Rémi Duraffort | ivoire http://ivoire.dinauz.org/blog/
On Tuesday 19 October 2010, Rémi Duraffort wrote:
Hello,
- free(ptl_mait->countries[i].pf_ptl_mai); + if (ptl_mait->countries[i].pf_ptl_mai) + free(ptl_mait->countries[i].pf_ptl_mai);
AFAIK free(NULL) is totally valid so this change is just a non-op.
Thanks for pointing it out. It wasn't safe when I started programming in C... I missed that 20 years old evolution :-). Frederic
Hi, On Saturday, 16 October 2010 at 11:21, Frédéric Marchal wrote:
Hello,
On July 11th, Morten Sjøgren reported a segfault due to the double free of a buffer in ifoFree_PTL_MAIT and provided a patch:
http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/2010-July/066025.html
The patch was taken into account by Dominik 'Rathann' Mierzejewski on September 2nd:
http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/2010-September/066033.ht...
The patch is incomplete as ifoRead_PTL_MAIT can still return without resetting the pointer in ifofile->ptl_mait. In my case, I have two DVDs failing due an invalid seek offset passed to DVDFileSeek_().
The following patch set ifofile->ptl_mait to NULL (not 0 as in the original patch) before every return statement.
Moreover, if the seek offset is obviously out of range, the loop over the countries is interrupted and the nr_of_countries is set to the index of the highest country that could be read. I'm not sure it is the proper action but without a test case to disprove it, it sound sensible as nr_of_countries doesn't seem to be used.
I think this has been at least partially applied. If there are still any issues that this patch fixes, could you resend a patch against current SVN? Regards, Dominik -- Fedora http://fedoraproject.org/wiki/User:Rathann RPMFusion http://rpmfusion.org | MPlayer http://mplayerhq.hu "Faith manages." -- Delenn to Lennier in Babylon 5:"Confessions and Lamentations"
On Thursday 06 October 2011, Dominik 'Rathann' Mierzejewski wrote:
Hi,
On Saturday, 16 October 2010 at 11:21, Frédéric Marchal wrote:
Hello,
On July 11th, Morten Sjøgren reported a segfault due to the double free of a buffer in ifoFree_PTL_MAIT and provided a patch:
http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/2010-July/066025.html
The patch was taken into account by Dominik 'Rathann' Mierzejewski on September 2nd:
http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/2010-September/066033 .html
The patch is incomplete as ifoRead_PTL_MAIT can still return without resetting the pointer in ifofile->ptl_mait. In my case, I have two DVDs failing due an invalid seek offset passed to DVDFileSeek_().
The following patch set ifofile->ptl_mait to NULL (not 0 as in the original patch) before every return statement.
Moreover, if the seek offset is obviously out of range, the loop over the countries is interrupted and the nr_of_countries is set to the index of the highest country that could be read. I'm not sure it is the proper action but without a test case to disprove it, it sound sensible as nr_of_countries doesn't seem to be used.
I think this has been at least partially applied. If there are still any issues that this patch fixes, could you resend a patch against current SVN?
The part with the ifofile->ptl_mait is fixed. Note that on a purely cosmetic point of view, some ifofile->ptl_mait are reset to 0 (zero) and others to NULL. The second patch with the capping of nr_of_countries was never applied as I couldn't remember, two months after submitting it, which dvd displayed the problem the patch was supposed to address. Frederic
On Thursday, 06 October 2011 at 16:23, Frédéric Marchal wrote:
On Thursday 06 October 2011, Dominik 'Rathann' Mierzejewski wrote:
Hi,
I think this has been at least partially applied. If there are still any issues that this patch fixes, could you resend a patch against current SVN?
The part with the ifofile->ptl_mait is fixed. Note that on a purely cosmetic point of view, some ifofile->ptl_mait are reset to 0 (zero) and others to NULL.
The cosmetic changes could still be done (if only for consistency's sake). If you have some spare time, please send a patch doing just that and I'll apply it.
The second patch with the capping of nr_of_countries was never applied as I couldn't remember, two months after submitting it, which dvd displayed the problem the patch was supposed to address.
OK, thanks for the update. Dropping from my list, then. Regards, Dominik -- Fedora http://fedoraproject.org/wiki/User:Rathann RPMFusion http://rpmfusion.org | MPlayer http://mplayerhq.hu "Faith manages." -- Delenn to Lennier in Babylon 5:"Confessions and Lamentations"
On Friday 07 October 2011, Dominik 'Rathann' Mierzejewski wrote:
On Thursday, 06 October 2011 at 16:23, Frédéric Marchal wrote:
On Thursday 06 October 2011, Dominik 'Rathann' Mierzejewski wrote:
Hi,
I think this has been at least partially applied. If there are still any issues that this patch fixes, could you resend a patch against current SVN?
The part with the ifofile->ptl_mait is fixed. Note that on a purely cosmetic point of view, some ifofile->ptl_mait are reset to 0 (zero) and others to NULL.
The cosmetic changes could still be done (if only for consistency's sake). If you have some spare time, please send a patch doing just that and I'll apply it.
Here is the patch to reset ptl_mait with NULL instead of 0 and compare it against NULL instead of 0. Frederic
Here is the patch to reset ptl_mait with NULL instead of 0 and compare it against NULL instead of 0.
Looks great. Applied. Thanks E -- Erik Hovland erik@hovland.org http://hovland.org/
participants (4)
-
Dominik 'Rathann' Mierzejewski -
Erik Hovland -
Frédéric Marchal -
Rémi Duraffort