r1018 - in trunk/libdvdnav/src: navigation.c vm/vm.c
Author: nicodvb Date: Sun Apr 20 10:29:02 2008 New Revision: 1018 Log: in get_PGCIT() check the validity of vm->vtsi before risking to dereference NULL; also change dvdnav_get_current_menu() accordingly\npatch by Erik Hovland - erik hovland org Modified: trunk/libdvdnav/src/navigation.c trunk/libdvdnav/src/vm/vm.c Modified: trunk/libdvdnav/src/navigation.c ============================================================================== --- trunk/libdvdnav/src/navigation.c (original) +++ trunk/libdvdnav/src/navigation.c Sun Apr 20 10:29:02 2008 @@ -104,7 +104,10 @@ dvdnav_status_t dvdnav_current_title_inf if ( (this->vm->state.domain == VTSM_DOMAIN) || (this->vm->state.domain == VMGM_DOMAIN) ) { /* Get current Menu ID: into *part. */ - vm_get_current_menu(this->vm, part); + if(! vm_get_current_menu(this->vm, part)) { + pthread_mutex_unlock(&this->vm_lock); + return DVDNAV_STATUS_ERR; + } if (*part > -1) { *title = 0; pthread_mutex_unlock(&this->vm_lock); Modified: trunk/libdvdnav/src/vm/vm.c ============================================================================== --- trunk/libdvdnav/src/vm/vm.c (original) +++ trunk/libdvdnav/src/vm/vm.c Sun Apr 20 10:29:02 2008 @@ -1823,13 +1823,15 @@ static pgcit_t* get_MENU_PGCIT(vm_t *vm, /* Uses state to decide what to return */ static pgcit_t* get_PGCIT(vm_t *vm) { - pgcit_t *pgcit; + pgcit_t *pgcit = NULL; switch ((vm->state).domain) { case VTS_DOMAIN: + if(!vm->vtsi) return NULL; pgcit = vm->vtsi->vts_pgcit; break; case VTSM_DOMAIN: + if(!vm->vtsi) return NULL; pgcit = get_MENU_PGCIT(vm, vm->vtsi, (vm->state).registers.SPRM[0]); break; case VMGM_DOMAIN:
Il Sunday 20 April 2008 10:29:03 nicodvb ha scritto:
Author: nicodvb Date: Sun Apr 20 10:29:02 2008 New Revision: 1018
Log: in get_PGCIT() check the validity of vm->vtsi before risking to dereference NULL; also change dvdnav_get_current_menu() accordingly\npatch by Erik Hovland - erik hovland org
oops, my little trick didn't work as expected :)
On Sun, Apr 20, 2008 at 10:25:09AM +0200, Nico Sabbi wrote:
Il Sunday 20 April 2008 10:29:03 nicodvb ha scritto:
Author: nicodvb Date: Sun Apr 20 10:29:02 2008 New Revision: 1018
Log: in get_PGCIT() check the validity of vm->vtsi before risking to dereference NULL; also change dvdnav_get_current_menu() accordingly\npatch by Erik Hovland - erik hovland org
oops, my little trick didn't work as expected :)
Ah, you used commit -m? I was already wondering how you managed to do _that_ ;-)
Il Sunday 20 April 2008 10:32:10 Reimar Döffinger ha scritto:
oops, my little trick didn't work as expected :)
Ah, you used commit -m? I was already wondering how you managed to do _that_ ;-)
yes, as Diego says I'm "iron-clang" to my habits :)
On Sun, Apr 20, 2008 at 10:29:03AM +0200, nicodvb wrote:
Author: nicodvb Date: Sun Apr 20 10:29:02 2008 New Revision: 1018
Log: in get_PGCIT() check the validity of vm->vtsi before risking to dereference NULL; also change dvdnav_get_current_menu() accordingly patch by Erik Hovland - erik hovland org
The navigation.c catch is very good. I think the fix is incomplete though. Because the current source has: int vm_get_current_menu(vm_t *vm, int *menuid) { pgcit_t* pgcit; int pgcn; pgcn = (vm->state).pgcN; pgcit = get_PGCIT(vm); *menuid = pgcit->pgci_srp[pgcn - 1].entry_id & 0xf ; return 1; } While my original patch added the line: int vm_get_current_menu(vm_t *vm, int *menuid) { pgcit_t* pgcit; int pgcn; pgcn = (vm->state).pgcN; pgcit = get_PGCIT(vm); if (pgcit == NULL) return 0; *menuid = pgcit->pgci_srp[pgcn - 1].entry_id & 0xf ; return 1; } Without the if clause to return 0, then vm_get_current_menu will cause any program that uses that function to segfault on the next line with the NULL pgcit is dereferenced. Let me know if I am totally off of my rocker. E src/vm/vm.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/vm/vm.c b/src/vm/vm.c index 129b330..996f0ea 100644 --- a/src/vm/vm.c +++ b/src/vm/vm.c @@ -612,6 +612,7 @@ int vm_get_current_menu(vm_t *vm, int *menuid) { int pgcn; pgcn = (vm->state).pgcN; pgcit = get_PGCIT(vm); + if (pgcit == NULL) return 0; *menuid = pgcit->pgci_srp[pgcn - 1].entry_id & 0xf ; return 1; } -- Erik Hovland mail: erik@hovland.org web: http://hovland.org/ PGP/GPG public key available on request
On Tuesday 22 April 2008 00:34:11 Erik Hovland wrote:
Without the if clause to return 0, then vm_get_current_menu will cause any program that uses that function to segfault on the next line with the NULL pgcit is dereferenced.
Let me know if I am totally off of my rocker.
E
src/vm/vm.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/vm/vm.c b/src/vm/vm.c index 129b330..996f0ea 100644 --- a/src/vm/vm.c +++ b/src/vm/vm.c @@ -612,6 +612,7 @@ int vm_get_current_menu(vm_t *vm, int *menuid) { int pgcn; pgcn = (vm->state).pgcN; pgcit = get_PGCIT(vm); + if (pgcit == NULL) return 0; *menuid = pgcit->pgci_srp[pgcn - 1].entry_id & 0xf ; return 1; }
you are correct, committed
participants (4)
-
Erik Hovland -
Nico Sabbi -
nicodvb -
Reimar Döffinger