John Stebbins <[email protected]> wrote:

> On 11/04/2012 02:51 PM, Fabian Keil wrote:
> > John Stebbins <[email protected]> wrote:
> >
> >> This patch never got accepted (probably because it's touches a lot and
> >> made you nervous), but HandBrake has been applying this for 3 years
> >> now.
> >>
> >> Some discs have large numbers of repeated language units and pgc's
> >> that cause extreme memory consumption and file i/o issues. scanning
> >> all the titles on such discs is unusably slow. detect such repeats
> >> and ref-count the data structures to avoid reading and storing
> >> duplicate info.
> > A couple of days ago dvdbackup got killed after the system ran out of
> > swap space (2 GB), I applied this patch and it solved the problem.
> > Awesome.
> >
> > While I assumed that the changes from malloc() to calloc() might
> > also make crashes due to bogus pgc field values less likely, the
> > patch actually causes at least one crash that doesn't happen without
> > it:
> >
> > Program terminated with signal 10, Bus error.
> > #0  0x0000000802a2390a in dvdnav_describe_title_chapters
> > (this=0x80cd03e00, title=17, times=0x7fffff8fa298,
> > duration=0x7fffff8fa2a0)
> > at 
> > /usr/obj-ports/usr/ports/multimedia/libdvdnav/work/libdvdnav-4.2.0/src/searching.c:628
> > 628     if(ptt[i].pgn > pgc->nr_of_programs) { (gdb) p *pgc
> > Cannot access memory at address 0xe000e000000a620
> >
> > Everyone's favourite function dvdnav_describe_title_chapters() strikes
> > again ...
> >
> > I haven't tracked down the cause yet.
> >
> > Letting the dup_* functions always return -1 doesn't seem to affect the
> > problem and neither does modifying find_dup_pgc() to compare the whole
> > pgc instead of just the start_byte or removing the added return in
> > ifoRead_VTS_PTT_SRPT().
> >
> > As far as I can tell the DVD doesn't actually have duplicated pgcs.

> What disc does it crash on?  HandBrake does not use
> dvdnav_describe_title_chapters(), so this is a code path we have not
> tested well enough it sounds like.  dvdbackup uses this?

dvdbackup doesn't use dvdnav_describe_title_chapters() either.

I failed to mention that the crashing application is vlc.

The DVD is a rip of Monty Python season 3 DVD 1 where unreadable
sector regions have been padded with surrounding sectors using a
flawed padding strategy, which probably made the "copy" even less
standard-compliant than the original.

Given the content, it seems very appropriate that the attached patch,
which prevents this and other crashes, is somewhat silly ...

I made the skip conditions up while going through backtraces until
vlc stopped crashing. I haven't properly tested yet how they affect
other DVDs.

Fabian
From 910e08034f29b35563b695971c8713a5da72fbb2 Mon Sep 17 00:00:00 2001
From: Fabian Keil <[email protected]>
Date: Sun, 4 Nov 2012 18:03:33 +0100
Subject: [PATCH] Add a couple of additional sanity checks for
 dvdnav_describe_title_chapters()

Fixes crashes with non-compliant DVDs after applying
the duplicate detection patch for libdvdread.

It might make more sense to do those checks in libdvdread
instead and zero out structures that don't check out.
---
 src/searching.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/searching.c b/src/searching.c
index 4c3fb2f..c044199 100644
--- a/src/searching.c
+++ b/src/searching.c
@@ -620,6 +620,18 @@ uint32_t dvdnav_describe_title_chapters(dvdnav_t *this, int32_t title, uint64_t
       printerr("PGC start out of bounds");
       continue;
     }
+    if (0 == ifo->vts_pgcit->pgci_srp[ptt[i].pgcn-1].pgc_start_byte) {
+      printerr("PGC start zero.");
+      continue;
+    }
+    if (0 != (ifo->vts_pgcit->pgci_srp[ptt[i].pgcn-1].pgc_start_byte & 1)) {
+      printerr("PGC start unaligned.");
+      continue;
+    }
+    if (0 != ((int)(ifo->vts_pgcit->pgci_srp[ptt[i].pgcn-1].pgc) & 1)) {
+      printerr("PGC pointer unaligned.");
+      continue;
+    }
     pgc = ifo->vts_pgcit->pgci_srp[ptt[i].pgcn-1].pgc;
     if (pgc == NULL) {
       printerr("PGC missing.");
-- 
1.8.0

Attachment: signature.asc
Description: PGP signature

_______________________________________________
DVDnav-discuss mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/dvdnav-discuss

Reply via email to