Cyril Brulebois <kibi@debian.org> (30/11/2010): > (gdb) bt full > #0 0x0effde04 in read_legacy_video_BIOS (entityIndex=0, Flags=<value optimized out>) at ../../../../hw/xfree86/int10/generic.c:105 > len = 0 > pagemask = <value optimized out> > offset = 786432 > size = 131072 > ptr = 0x4816b000 <Address 0x4816b000 out of bounds> > src = 0x4816b000 <Address 0x4816b000 out of bounds> Lalala: (gdb) p dev $4 = <value optimized out> (gdb) p !dev $5 = 1 So maybe read_legacy_video_BIOS() or its caller should error out long before that happens? Oh, wait, in the caller: | /* FIXME: Shouldn't this be a failure case? Leaving dev as NULL seems like | * FIXME: an error | */ | pInt->dev = xf86GetPciInfoForEntity(entityIndex); I'm attaching a (currently untested) patch, which does two things: - create a new error2 label, which replaces the error1 one, keeping "UnmapVRam(pInt);" out of it; and replace all "goto error1;" with "goto error2;" accordingly. - check for NULL and "goto error1;" accordingly; indeed, "MapVRam(pInt);" hasn't been called at this stage. If that turns out to work fine, it'll probably be included at two separate patches. I'd be glad if you could give this patch (against the xorg-server package) a try. Mraw, KiBi.
From 0839f0f546a0a798b3e13e57ac73ec717c605832 Mon Sep 17 00:00:00 2001
From: Cyril Brulebois <kibi@debian.org>
Date: Tue, 30 Nov 2010 02:58:26 +0100
Subject: [PATCH] xfree86: Error out if xf86GetPciInfoForEntity() returns no device.
As noted in the comment, things can go very wrong if the device is NULL,
e.g. through the read_legacy_video_BIOS() then xf86MapDomainMemory()
codepath, the latter returning bad pointers to the former, leading to
bus errors on powerpc for example.
Reported-by: Alexander Tait Brotman <atbrotman@yahoo.com>
Signed-off-by: Cyril Brulebois <kibi@debian.org>
---
hw/xfree86/int10/generic.c | 18 +++++++++---------
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/hw/xfree86/int10/generic.c b/hw/xfree86/int10/generic.c
index 9d39e99..40fa2a4 100644
--- a/hw/xfree86/int10/generic.c
+++ b/hw/xfree86/int10/generic.c
@@ -151,10 +151,9 @@ xf86ExtendedInitInt10(int entityIndex, int Flags)
pInt->scrnIndex = screen;
base = INTPriv(pInt)->base = xnfalloc(SYS_BIOS);
- /* FIXME: Shouldn't this be a failure case? Leaving dev as NULL seems like
- * FIXME: an error
- */
pInt->dev = xf86GetPciInfoForEntity(entityIndex);
+ if (pInt->dev == NULL)
+ goto error1;
/*
* we need to map video RAM MMIO as some chipsets map mmio
@@ -169,7 +168,7 @@ xf86ExtendedInitInt10(int entityIndex, int Flags)
if (xf86ReadBIOS(0, 0, base, LOW_PAGE_SIZE) < 0) {
xf86DrvMsg(screen, X_ERROR, "Cannot read int vect\n");
- goto error1;
+ goto error2;
}
/*
@@ -189,7 +188,7 @@ xf86ExtendedInitInt10(int entityIndex, int Flags)
if (xf86IsEntityPrimary(entityIndex) && !(initPrimary(options))) {
if (!xf86int10GetBiosSegment(pInt, (unsigned char *)sysMem - V_BIOS))
- goto error1;
+ goto error2;
set_return_trap(pInt);
@@ -216,13 +215,13 @@ xf86ExtendedInitInt10(int entityIndex, int Flags)
if (err) {
xf86DrvMsg(screen,X_ERROR,"Cannot read V_BIOS (3) %s\n",
strerror(err));
- goto error1;
+ goto error2;
}
INTPriv(pInt)->highMemory = GET_HIGH_BASE(rom_device->rom_size);
break;
}
default:
- goto error1;
+ goto error2;
}
pInt->BIOSseg = V_BIOS >> 4;
pInt->num = 0xe6;
@@ -274,7 +273,7 @@ xf86ExtendedInitInt10(int entityIndex, int Flags)
if (err) {
xf86DrvMsg(screen,X_ERROR,"Cannot read V_BIOS (5) %s\n",
strerror(err));
- goto error1;
+ goto error2;
}
}
}
@@ -288,9 +287,10 @@ xf86ExtendedInitInt10(int entityIndex, int Flags)
xfree(options);
return pInt;
+ error2:
+ UnmapVRam(pInt);
error1:
xfree(base);
- UnmapVRam(pInt);
xfree(INTPriv(pInt)->alloc);
xfree(pInt->private);
error0:
--
1.7.2.3
Attachment:
signature.asc
Description: Digital signature