From 190644e180794208bc638179f4d5940fe419bf9c Mon Sep 17 00:00:00 2001 From: Jesper Juhl Date: Sat, 21 Jul 2007 17:39:11 +0200 Subject: [PATCH 1/5] Fix "use after free" / "double free" bug in ati_create_gatt_pages / ati_free_gatt_pages Hi, Coverity spotted a "use after free" bug in drivers/char/agp/ati-agp.c::ati_create_gatt_pages(). The same one that was in drivers/char/agp/amd-k7-agp.c::amd_create_gatt_pages() The problem is this: If "entry = kzalloc(sizeof(struct ati_page_map), GFP_KERNEL);" fails, then there's a loop in the function to free all entries allocated so far and break out of the allocation loop. That in itself is pretty sane, but then the (now freed) 'tables' is assigned to ati_generic_private.gatt_pages and 'retval' is set to -ENOMEM which causes ati_free_gatt_pages(); to be called at the end of the function. The problem with this is that ati_free_gatt_pages() will then loop 'ati_generic_private.num_tables' times and try to free each entry in tables[] - this is bad since tables has already been freed and furthermore it will call kfree(tables) at the end - a double free. This patch removes the freeing loop in ati_create_gatt_pages() and instead relies entirely on the call to ati_free_gatt_pages() to free everything we allocated in case of an error. It also sets ati_generic_private.num_tables to the actual number of entries allocated instead of just using the value passed in from the caller - this ensures that ati_free_gatt_pages() will only attempt to free stuff that was actually allocated. Note: I'm in no way intimate with this code and I have no way to actually test this patch (besides compile test it), so while I've tried to be careful in reading the code and make sure the patch does the right thing an ACK from someone who actually knows the code in-depth would be very much appreciated. Signed-off-by: Jesper Juhl Signed-off-by: Dave Airlie --- drivers/char/agp/ati-agp.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/char/agp/ati-agp.c b/drivers/char/agp/ati-agp.c index 780e59e588ad..da7513d7b4e7 100644 --- a/drivers/char/agp/ati-agp.c +++ b/drivers/char/agp/ati-agp.c @@ -123,21 +123,16 @@ static int ati_create_gatt_pages(int nr_tables) for (i = 0; i < nr_tables; i++) { entry = kzalloc(sizeof(struct ati_page_map), GFP_KERNEL); + tables[i] = entry; if (entry == NULL) { - while (i > 0) { - kfree(tables[i-1]); - i--; - } - kfree(tables); retval = -ENOMEM; break; } - tables[i] = entry; retval = ati_create_page_map(entry); if (retval != 0) break; } - ati_generic_private.num_tables = nr_tables; + ati_generic_private.num_tables = i; ati_generic_private.gatt_pages = tables; if (retval != 0) From c99c108ac362f5cc37f79fad7e9897bd9d033bcc Mon Sep 17 00:00:00 2001 From: Chuck Ebbert Date: Fri, 27 Jul 2007 10:46:20 +1000 Subject: [PATCH 2/5] AGP: document boot options Add documentation for AGP boot options. Signed-off-by: Chuck Ebbert Signed-off-by: Dave Airlie --- Documentation/kernel-parameters.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index fb80e9ffea68..1156653338fe 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -30,6 +30,7 @@ the beginning of each description states the restrictions within which a parameter is applicable: ACPI ACPI support is enabled. + AGP AGP (Accelerated Graphics Port) is enabled. ALSA ALSA sound support is enabled. APIC APIC support is enabled. APM Advanced Power Management support is enabled. @@ -227,6 +228,12 @@ and is between 256 and 4096 characters. It is defined in the file to assume that this machine's pmtimer latches its value and always returns good values. + agp= [AGP] + { off | try_unsupported } + off: disable AGP support + try_unsupported: try to drive unsupported chipsets + (may crash computer or cause data corruption) + enable_timer_pin_1 [i386,x86-64] Enable PIN 1 of APIC timer Can be useful to work around chipset bugs From a51b34593f691a0837d752a1394dcee19483c607 Mon Sep 17 00:00:00 2001 From: Nick Piggin Date: Wed, 25 Jul 2007 13:19:22 +0200 Subject: [PATCH 3/5] agp: don't lock pages AGP should not need to lock pages. They are not protecting any race because there is no lock_page calls, only SetPageLocked. This is causing hangs with d00806b183152af6d24f46f0c33f14162ca1262a. Signed-off-by: Nick Piggin Signed-off-by: Dave Airlie --- drivers/char/agp/generic.c | 2 -- drivers/char/agp/intel-agp.c | 2 -- drivers/char/agp/sgi-agp.c | 1 - 3 files changed, 5 deletions(-) diff --git a/drivers/char/agp/generic.c b/drivers/char/agp/generic.c index d535c406b319..3db4f4076ed4 100644 --- a/drivers/char/agp/generic.c +++ b/drivers/char/agp/generic.c @@ -1170,7 +1170,6 @@ void *agp_generic_alloc_page(struct agp_bridge_data *bridge) map_page_into_agp(page); get_page(page); - SetPageLocked(page); atomic_inc(&agp_bridge->current_memory_agp); return page_address(page); } @@ -1187,7 +1186,6 @@ void agp_generic_destroy_page(void *addr) page = virt_to_page(addr); unmap_page_from_agp(page); put_page(page); - unlock_page(page); free_page((unsigned long)addr); atomic_dec(&agp_bridge->current_memory_agp); } diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c index a1240603912c..2f319f474f70 100644 --- a/drivers/char/agp/intel-agp.c +++ b/drivers/char/agp/intel-agp.c @@ -213,7 +213,6 @@ static void *i8xx_alloc_pages(void) } global_flush_tlb(); get_page(page); - SetPageLocked(page); atomic_inc(&agp_bridge->current_memory_agp); return page_address(page); } @@ -229,7 +228,6 @@ static void i8xx_destroy_pages(void *addr) change_page_attr(page, 4, PAGE_KERNEL); global_flush_tlb(); put_page(page); - unlock_page(page); __free_pages(page, 2); atomic_dec(&agp_bridge->current_memory_agp); } diff --git a/drivers/char/agp/sgi-agp.c b/drivers/char/agp/sgi-agp.c index cda608c42bea..98cf8abb3e57 100644 --- a/drivers/char/agp/sgi-agp.c +++ b/drivers/char/agp/sgi-agp.c @@ -51,7 +51,6 @@ static void *sgi_tioca_alloc_page(struct agp_bridge_data *bridge) return NULL; get_page(page); - SetPageLocked(page); atomic_inc(&agp_bridge->current_memory_agp); return page_address(page); } From dde4787642ee3cb85aef80bdade04b6f8ddc3df8 Mon Sep 17 00:00:00 2001 From: Zhenyu Wang Date: Thu, 26 Jul 2007 09:18:09 +0800 Subject: [PATCH 4/5] intel_agp: really fix 945/965GME Fix some missing places to check with device id info, which should probe the device gart correctly. Signed-off-by: Wang Zhenyu Signed-off-by: Dave Airlie --- drivers/char/agp/intel-agp.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c index 2f319f474f70..294cdbf4d44d 100644 --- a/drivers/char/agp/intel-agp.c +++ b/drivers/char/agp/intel-agp.c @@ -20,7 +20,9 @@ #define PCI_DEVICE_ID_INTEL_82965G_IG 0x29A2 #define PCI_DEVICE_ID_INTEL_82965GM_HB 0x2A00 #define PCI_DEVICE_ID_INTEL_82965GM_IG 0x2A02 +#define PCI_DEVICE_ID_INTEL_82965GME_HB 0x2A10 #define PCI_DEVICE_ID_INTEL_82965GME_IG 0x2A12 +#define PCI_DEVICE_ID_INTEL_82945GME_HB 0x27AC #define PCI_DEVICE_ID_INTEL_82945GME_IG 0x27AE #define PCI_DEVICE_ID_INTEL_G33_HB 0x29C0 #define PCI_DEVICE_ID_INTEL_G33_IG 0x29C2 @@ -33,7 +35,8 @@ agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82965G_1_HB || \ agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82965Q_HB || \ agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82965G_HB || \ - agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82965GM_HB) + agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82965GM_HB || \ + agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82965GME_HB) #define IS_G33 (agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_G33_HB || \ agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_Q35_HB || \ @@ -525,6 +528,7 @@ static void intel_i830_init_gtt_entries(void) agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82915GM_HB || agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82945G_HB || agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82945GM_HB || + agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82945GME_HB || IS_I965 || IS_G33) gtt_entries = MB(48) - KB(size); else @@ -536,6 +540,7 @@ static void intel_i830_init_gtt_entries(void) agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82915GM_HB || agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82945G_HB || agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82945GM_HB || + agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82945GME_HB || IS_I965 || IS_G33) gtt_entries = MB(64) - KB(size); else @@ -1846,9 +1851,9 @@ static const struct intel_driver_description { NULL, &intel_915_driver }, { PCI_DEVICE_ID_INTEL_82945G_HB, PCI_DEVICE_ID_INTEL_82945G_IG, 0, "945G", NULL, &intel_915_driver }, - { PCI_DEVICE_ID_INTEL_82945GM_HB, PCI_DEVICE_ID_INTEL_82945GM_IG, 1, "945GM", + { PCI_DEVICE_ID_INTEL_82945GM_HB, PCI_DEVICE_ID_INTEL_82945GM_IG, 0, "945GM", NULL, &intel_915_driver }, - { PCI_DEVICE_ID_INTEL_82945GM_HB, PCI_DEVICE_ID_INTEL_82945GME_IG, 0, "945GME", + { PCI_DEVICE_ID_INTEL_82945GME_HB, PCI_DEVICE_ID_INTEL_82945GME_IG, 0, "945GME", NULL, &intel_915_driver }, { PCI_DEVICE_ID_INTEL_82946GZ_HB, PCI_DEVICE_ID_INTEL_82946GZ_IG, 0, "946GZ", NULL, &intel_i965_driver }, @@ -1858,9 +1863,9 @@ static const struct intel_driver_description { NULL, &intel_i965_driver }, { PCI_DEVICE_ID_INTEL_82965G_HB, PCI_DEVICE_ID_INTEL_82965G_IG, 0, "965G", NULL, &intel_i965_driver }, - { PCI_DEVICE_ID_INTEL_82965GM_HB, PCI_DEVICE_ID_INTEL_82965GM_IG, 1, "965GM", + { PCI_DEVICE_ID_INTEL_82965GM_HB, PCI_DEVICE_ID_INTEL_82965GM_IG, 0, "965GM", NULL, &intel_i965_driver }, - { PCI_DEVICE_ID_INTEL_82965GM_HB, PCI_DEVICE_ID_INTEL_82965GME_IG, 0, "965GME/GLE", + { PCI_DEVICE_ID_INTEL_82965GME_HB, PCI_DEVICE_ID_INTEL_82965GME_IG, 0, "965GME/GLE", NULL, &intel_i965_driver }, { PCI_DEVICE_ID_INTEL_7505_0, 0, 0, "E7505", &intel_7505_driver, NULL }, { PCI_DEVICE_ID_INTEL_7205_0, 0, 0, "E7205", &intel_7505_driver, NULL }, @@ -2049,11 +2054,13 @@ static struct pci_device_id agp_intel_pci_table[] = { ID(PCI_DEVICE_ID_INTEL_82915GM_HB), ID(PCI_DEVICE_ID_INTEL_82945G_HB), ID(PCI_DEVICE_ID_INTEL_82945GM_HB), + ID(PCI_DEVICE_ID_INTEL_82945GME_HB), ID(PCI_DEVICE_ID_INTEL_82946GZ_HB), ID(PCI_DEVICE_ID_INTEL_82965G_1_HB), ID(PCI_DEVICE_ID_INTEL_82965Q_HB), ID(PCI_DEVICE_ID_INTEL_82965G_HB), ID(PCI_DEVICE_ID_INTEL_82965GM_HB), + ID(PCI_DEVICE_ID_INTEL_82965GME_HB), ID(PCI_DEVICE_ID_INTEL_G33_HB), ID(PCI_DEVICE_ID_INTEL_Q35_HB), ID(PCI_DEVICE_ID_INTEL_Q33_HB), From f191f144079b0083c6fa7d01a4acbd7263fb5032 Mon Sep 17 00:00:00 2001 From: Alan Hourihane Date: Fri, 27 Jul 2007 10:56:43 +1000 Subject: [PATCH 5/5] agp: AMD AGP is used on UP1100 & UP1500 alpha boxen Signed-off-by: Dave Airlie --- drivers/char/agp/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/char/agp/Kconfig b/drivers/char/agp/Kconfig index a9f9c48c2424..713533d8a86e 100644 --- a/drivers/char/agp/Kconfig +++ b/drivers/char/agp/Kconfig @@ -50,7 +50,7 @@ config AGP_ATI config AGP_AMD tristate "AMD Irongate, 761, and 762 chipset support" - depends on AGP && X86_32 + depends on AGP && (X86_32 || ALPHA) help This option gives you AGP support for the GLX component of X on AMD Irongate, 761, and 762 chipsets.