[patch] Fix SIGSEGV in dvdnav_describe_title_chapters
Hi, Please consider checking and applying the attached patch. It fixes a segfault occuring with some DVDs. It seems to be the same problem as the one mentionned here: https://bugs.launchpad.net/ubuntu/+source/libdvdnav/+bug/934471 In my case, "cellnr" and "endcellnr" variables were set to 0 when the crash occured. Cheers Sylvain PS: I'm not registered on the mailing-list, please answer directly to me if necessary
On Mon, Apr 2, 2012 at 12:16 PM, Sylvain HENRY <hsyl20@gmail.com> wrote:
Hi,
Please consider checking and applying the attached patch. It fixes a segfault occuring with some DVDs. It seems to be the same problem as the one mentionned here: https://bugs.launchpad.net/ubuntu/+source/libdvdnav/+bug/934471
In my case, "cellnr" and "endcellnr" variables were set to 0 when the crash occured.
Although I don't doubt that your patch fixes the problem for you it seems to address a different problem. The crash is likely because: cell = &pgc->cell_playback[cellnr-1]; In this case cellnr should never be zero because cellnr-1 will evaluate to UINT_MAX. A patch that is more likely to be accepted is one that checks cellnr. What does the attached patch do? E -- Erik Hovland erik@hovland.org http://hovland.org/
My patch checks that cellnr is strictly inferior to endcellnr. It was only based on the fact that this condition was true for every case except the bogus case (cellnr=0 and endcellnr=0). If my assumption is wrong, a patch that excludes the case "cellnr=0" would do the trick. Cheers Sylvain Le 03/04/2012 20:08, Erik Hovland a écrit :
On Mon, Apr 2, 2012 at 12:16 PM, Sylvain HENRY<hsyl20@gmail.com> wrote:
Hi,
Please consider checking and applying the attached patch. It fixes a segfault occuring with some DVDs. It seems to be the same problem as the one mentionned here: https://bugs.launchpad.net/ubuntu/+source/libdvdnav/+bug/934471
In my case, "cellnr" and "endcellnr" variables were set to 0 when the crash occured. Although I don't doubt that your patch fixes the problem for you it seems to address a different problem. The crash is likely because: cell =&pgc->cell_playback[cellnr-1];
In this case cellnr should never be zero because cellnr-1 will evaluate to UINT_MAX. A patch that is more likely to be accepted is one that checks cellnr. What does the attached patch do?
E
On Wed, Apr 4, 2012 at 5:56 AM, Sylvain HENRY <hsyl20@gmail.com> wrote:
My patch checks that cellnr is strictly inferior to endcellnr. It was only based on the fact that this condition was true for every case except the bogus case (cellnr=0 and endcellnr=0).
Your patch is not bad and does indeed address an issue that likely should be addressed. I like it. If cellnr = 0 and endcellnr = 1, will your patch stop a crash? E -- Erik Hovland erik@hovland.org http://hovland.org/
No it won't. But I have no idea about whether this case could happen or not as I haven't read any specification nor tried to understand the rest of the code. Thus, checking that cellnr != 0 may be a better fix. I let people who know better decide ;-) Cheers Sylvain Le 04/04/2012 16:48, Erik Hovland a écrit :
On Wed, Apr 4, 2012 at 5:56 AM, Sylvain HENRY<hsyl20@gmail.com> wrote:
My patch checks that cellnr is strictly inferior to endcellnr. It was only based on the fact that this condition was true for every case except the bogus case (cellnr=0 and endcellnr=0). Your patch is not bad and does indeed address an issue that likely should be addressed. I like it. If cellnr = 0 and endcellnr = 1, will your patch stop a crash?
E
On Wed, Apr 4, 2012 at 7:55 AM, Sylvain HENRY <hsyl20@gmail.com> wrote:
No it won't. But I have no idea about whether this case could happen or not as I haven't read any specification nor tried to understand the rest of the code.
Thus, checking that cellnr != 0 may be a better fix. I let people who know better decide ;-)
Please try the patch I sent in my original post so at least we know whether it works or not. Would you mind trying to get the other person on that launchpad bug to try it out as well? If we can get some people to try the patch I might commit it sooner rather then later. Thanks E -- Erik Hovland erik@hovland.org http://hovland.org/
Hi, Sorry for the delay. Your patch fixes the issue with my DVD. I have submitted the new patch on launchpad. Someone had already confirmed that my previous patch worked for him so I think yours will too. Cheers Sylvain Le 04/04/2012 16:58, Erik Hovland a écrit :
No it won't. But I have no idea about whether this case could happen or not as I haven't read any specification nor tried to understand the rest of the code.
Thus, checking that cellnr != 0 may be a better fix. I let people who know better decide ;-) Please try the patch I sent in my original post so at least we know whether it works or not. Would you mind trying to get the other person on that launchpad bug to try it out as well? If we can get some people to
On Wed, Apr 4, 2012 at 7:55 AM, Sylvain HENRY<hsyl20@gmail.com> wrote: try the patch I might commit it sooner rather then later.
Thanks
E
On Thu, Apr 12, 2012 at 1:16 PM, Sylvain HENRY <hsyl20@gmail.com> wrote:
Hi,
Sorry for the delay. Your patch fixes the issue with my DVD.
No problem. Thank you for taking the time to try it out.
I have submitted the new patch on launchpad. Someone had already confirmed that my previous patch worked for him so I think yours will too.
Since it worked for you, I have committed my patch and pushed it to my public libdvdnav tree: https://github.com/microe/libdvdnav/commit/49c67ccf88c688e0e0e9e3b04f651b12c... All of the commits in that tree will be pushed to svn soon. Thanks again for reporting the problem, submitting the original patch and then working w/ me on it. It is extremely helpful. E -- Erik Hovland erik@hovland.org http://hovland.org/
On Tuesday, 17 April 2012 at 00:07, Erik Hovland wrote:
On Thu, Apr 12, 2012 at 1:16 PM, Sylvain HENRY <hsyl20@gmail.com> wrote:
Sorry for the delay. Your patch fixes the issue with my DVD.
No problem. Thank you for taking the time to try it out.
I have submitted the new patch on launchpad. Someone had already confirmed that my previous patch worked for him so I think yours will too.
Since it worked for you, I have committed my patch and pushed it to my public libdvdnav tree: https://github.com/microe/libdvdnav/commit/49c67ccf88c688e0e0e9e3b04f651b12c...
All of the commits in that tree will be pushed to svn soon.
Thanks again for reporting the problem, submitting the original patch and then working w/ me on it. It is extremely helpful.
Applied to SVN, too. 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"
participants (3)
-
Dominik 'Rathann' Mierzejewski -
Erik Hovland -
Sylvain HENRY