From mboxrd@z Thu Jan 1 00:00:00 1970 From: Igor Murzov Subject: Re: [PATCH 1/2] drm/radeon: fix invalid memory access in radeon_atrm_get_bios() Date: Thu, 2 Feb 2012 01:22:37 +0400 Message-ID: <20120202012237.34bfa5f2@garik> References: <20120122184325.74528207@garik> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Dave Airlie Cc: Alex Deucher , David Airlie , Dave Airlie , Igor Murzov , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Sergey V List-Id: dri-devel@lists.freedesktop.org On Wed, 1 Feb 2012 18:42:52 +0000 Dave Airlie wrote: > On Tue, Jan 24, 2012 at 2:10 PM, Alex Deucher = wrote: > > On Sun, Jan 22, 2012 at 9:43 AM, Igor Murzov > > wrote: > >> From 77c912ea1eca50a93a34d5be69f9dc96a8bef0d8 Mon Sep 17 00:00:00 = 2001 > >> From: Igor Murzov > >> Date: Sun, 22 Jan 2012 19:02:27 +0400 > >> Subject: [PATCH 1/2] drm/radeon: fix invalid memory access in rade= on_atrm_get_bios() > >> > >> At a boot time I observed following bug: > >> > >> =C2=A0BUG: unable to handle kernel paging request at ffff8800a4244= 000 > >> =C2=A0IP: [] memcpy+0xb/0x120 > >> =C2=A0PGD 1816063 PUD 1fe7d067 PMD 1ff9f067 PTE 80000000a4244160 > >> =C2=A0Oops: 0000 [#1] SMP DEBUG_PAGEALLOC > >> =C2=A0CPU 0 > >> =C2=A0Modules linked in: btusb bluetooth brcmsmac brcmutil crc8 co= rdic b43 radeon(+) > >> =C2=A0mac80211 cfg80211 ttm ohci_hcd drm_kms_helper rfkill drm ssb= agpgart mmc_core > >> =C2=A0sp5100_tco video battery ac thermal processor rtc_cmos therm= al_sys snd_hda_codec_hdmi > >> =C2=A0joydev snd_hda_codec_conexant button bcma pcmcia snd_hda_int= el snd_hda_codec > >> =C2=A0snd_hwdep snd_pcm shpchp pcmcia_core k8temp snd_timer atl1c = snd psmouse hwmon > >> =C2=A0i2c_piix4 i2c_algo_bit soundcore evdev i2c_core ehci_hcd sg = serio_raw snd_page_alloc > >> =C2=A0loop btrfs > >> > >> =C2=A0Pid: 1008, comm: modprobe Not tainted 3.3.0-rc1 #21 LENOVO 2= 0046 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 /AMD CRB > >> =C2=A0RIP: 0010:[] =C2=A0[] me= mcpy+0xb/0x120 > >> =C2=A0RSP: 0018:ffff8800aa72db00 =C2=A0EFLAGS: 00010246 > >> =C2=A0RAX: ffff8800a4150000 RBX: 0000000000001000 RCX: 00000000000= 00087 > >> =C2=A0RDX: 0000000000000000 RSI: ffff8800a4244000 RDI: ffff8800a41= 50bc8 > >> =C2=A0RBP: ffff8800aa72db78 R08: 0000000000000010 R09: ffffffff817= 4bbec > >> =C2=A0R10: ffffffff812ee010 R11: 0000000000000001 R12: 00000000000= 01000 > >> =C2=A0R13: 0000000000010000 R14: ffff8800a4140000 R15: ffff8800aab= a1800 > >> =C2=A0FS: =C2=A000007ff9a3bd4720(0000) GS:ffff8800afa00000(0000) k= nlGS:0000000000000000 > >> =C2=A0CS: =C2=A00010 DS: 0000 ES: 0000 CR0: 000000008005003b > >> =C2=A0CR2: ffff8800a4244000 CR3: 00000000a9c18000 CR4: 00000000000= 006f0 > >> =C2=A0DR0: 0000000000000000 DR1: 0000000000000000 DR2: 00000000000= 00000 > >> =C2=A0DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 00000000000= 00400 > >> =C2=A0Process modprobe (pid: 1008, threadinfo ffff8800aa72c000, ta= sk ffff8800aa0e4000) > >> =C2=A0Stack: > >> =C2=A0ffffffffa04e7c7b 0000000000000001 0000000000010000 ffff8800a= a72db28 > >> =C2=A0ffffffff00000001 0000000000001000 ffffffff8113cbef 000000000= 0000020 > >> =C2=A0ffff8800a4243420 ffff880000000002 ffff8800aa72db08 ffff8800a= 9d42000 > >> =C2=A0Call Trace: > >> =C2=A0[] ? radeon_atrm_get_bios_chunk+0x8b/0xd0 = [radeon] > >> =C2=A0[] ? kmalloc_order_trace+0x3f/0xb0 > >> =C2=A0[] radeon_get_bios+0x68/0x2f0 [radeon] > >> =C2=A0[] rv770_init+0x40/0x280 [radeon] > >> =C2=A0[] radeon_device_init+0x560/0x600 [radeon] > >> =C2=A0[] radeon_driver_load_kms+0xaf/0x170 [rade= on] > >> =C2=A0[] drm_get_pci_dev+0x18e/0x2c0 [drm] > >> =C2=A0[] radeon_pci_probe+0xad/0xb5 [radeon] > >> =C2=A0[] local_pci_probe+0x5f/0xd0 > >> =C2=A0[] pci_device_probe+0x88/0xb0 > >> =C2=A0[] ? driver_sysfs_add+0x7a/0xb0 > >> =C2=A0[] really_probe+0x68/0x180 > >> =C2=A0[] driver_probe_device+0x45/0x70 > >> =C2=A0[] __driver_attach+0xa3/0xb0 > >> =C2=A0[] ? driver_probe_device+0x70/0x70 > >> =C2=A0[] bus_for_each_dev+0x5e/0x90 > >> =C2=A0[] driver_attach+0x1e/0x20 > >> =C2=A0[] bus_add_driver+0xc8/0x280 > >> =C2=A0[] driver_register+0x76/0x140 > >> =C2=A0[] __pci_register_driver+0x66/0xe0 > >> =C2=A0[] drm_pci_init+0x111/0x120 [drm] > >> =C2=A0[] ? vga_switcheroo_register_handler+0x3a/= 0x60 > >> =C2=A0[] ? 0xffffffffa0228fff > >> =C2=A0[] radeon_init+0xec/0xee [radeon] > >> =C2=A0[] do_one_initcall+0x42/0x180 > >> =C2=A0[] sys_init_module+0x92/0x1e0 > >> =C2=A0[] system_call_fastpath+0x16/0x1b > >> =C2=A0Code: 58 2a 43 50 88 43 4e 48 83 c4 08 5b c9 c3 66 90 e8 cb = fd ff ff eb > >> =C2=A0e6 90 90 90 90 90 90 90 90 90 48 89 f8 89 d1 c1 e9 03 83 e2 = 07 48 > >> =C2=A0a5 89 d1 f3 a4 c3 20 48 83 ea 20 4c 8b 06 4c 8b 4e 08 4c > >> =C2=A0RIP =C2=A0[] memcpy+0xb/0x120 > >> =C2=A0RSP > >> =C2=A0CR2: ffff8800a4244000 > >> =C2=A0---[ end trace fcffa1599cf56382 ]--- > >> > >> Call to acpi_evaluate_object() not always returns 4096 bytes chunk= s, > >> on my system it can return 2048 bytes chunk, so pass the length of > >> retrieved chunk to memcpy(), not the length of the recieving buffe= r. > >> > >> Signed-off-by: Igor Murzov >=20 > Hi Igor, >=20 > I'm not sure I understand, does your BIOS return 2K chunks always or > just for the last chunks? Only for the last chunk. acpi_evaluate_object() returns 16 x 4Kb chunks and then 1 x 2Kb on my laptop. If I revert both my patches (211fa4fc4e13492151e698d92b0dff56b29928ec a= nd a3f83ab1a717c0e6c2f59a4cfdaa10707cc35c55), applying following patch: ----------------------------------------- diff --git a/drivers/gpu/drm/radeon/radeon_atpx_handler.c b/drivers/gpu= /drm/radeon/radeon_atpx_handler.c index 9d95792..1376b94 100644 --- a/drivers/gpu/drm/radeon/radeon_atpx_handler.c +++ b/drivers/gpu/drm/radeon/radeon_atpx_handler.c @@ -58,6 +58,7 @@ static int radeon_atrm_call(acpi_handle atrm_handle, = uint8_t *bios, } =20 obj =3D (union acpi_object *)buffer.pointer; + printk(KERN_DEBUG "XXX len: %u, obj->buffer.length: %u", len, obj-= >buffer.length); memcpy(bios+offset, obj->buffer.pointer, len); kfree(buffer.pointer); return len; ----------------------------------------- gave me following result: ----------------------------------------- # dmesg | grep XXX =20 [ 10.895068] XXX len: 4096, obj->buffer.length: 4096 [ 10.998068] XXX len: 4096, obj->buffer.length: 4096 [ 11.101065] XXX len: 4096, obj->buffer.length: 4096 [ 11.204064] XXX len: 4096, obj->buffer.length: 4096 [ 11.307063] XXX len: 4096, obj->buffer.length: 4096 [ 11.410065] XXX len: 4096, obj->buffer.length: 4096 [ 11.513064] XXX len: 4096, obj->buffer.length: 4096 [ 11.616066] XXX len: 4096, obj->buffer.length: 4096 [ 11.722072] XXX len: 4096, obj->buffer.length: 4096 [ 11.826065] XXX len: 4096, obj->buffer.length: 4096 [ 11.929065] XXX len: 4096, obj->buffer.length: 4096 [ 12.032065] XXX len: 4096, obj->buffer.length: 4096 [ 12.135101] XXX len: 4096, obj->buffer.length: 4096 [ 12.239100] XXX len: 4096, obj->buffer.length: 4096 [ 12.343098] XXX len: 4096, obj->buffer.length: 4096 [ 12.468101] XXX len: 4096, obj->buffer.length: 2048 [ 12.552086] XXX len: 4096, obj->buffer.length: 1 [ 12.636085] XXX len: 4096, obj->buffer.length: 1 [ 12.720089] XXX len: 4096, obj->buffer.length: 1 [ 12.804085] XXX len: 4096, obj->buffer.length: 1 [ 12.888086] XXX len: 4096, obj->buffer.length: 1 [ 12.972088] XXX len: 4096, obj->buffer.length: 1 [ 13.056096] XXX len: 4096, obj->buffer.length: 1 [ 13.140085] XXX len: 4096, obj->buffer.length: 1 [ 13.224088] XXX len: 4096, obj->buffer.length: 1 [ 13.308084] XXX len: 4096, obj->buffer.length: 1 [ 13.392086] XXX len: 4096, obj->buffer.length: 1 [ 13.476089] XXX len: 4096, obj->buffer.length: 1 [ 13.560085] XXX len: 4096, obj->buffer.length: 1 [ 13.644085] XXX len: 4096, obj->buffer.length: 1 [ 13.728090] XXX len: 4096, obj->buffer.length: 1 [ 13.812085] XXX len: 4096, obj->buffer.length: 1 [ 13.896086] XXX len: 4096, obj->buffer.length: 1 [ 13.980088] XXX len: 4096, obj->buffer.length: 1 [ 14.064085] XXX len: 4096, obj->buffer.length: 1 [ 14.148085] XXX len: 4096, obj->buffer.length: 1 [ 14.232089] XXX len: 4096, obj->buffer.length: 1 [ 14.316084] XXX len: 4096, obj->buffer.length: 1 [ 14.400085] XXX len: 4096, obj->buffer.length: 1 [ 14.484089] XXX len: 4096, obj->buffer.length: 1 [ 14.568085] XXX len: 4096, obj->buffer.length: 1 [ 14.652085] XXX len: 4096, obj->buffer.length: 1 [ 14.736089] XXX len: 4096, obj->buffer.length: 1 [ 14.820084] XXX len: 4096, obj->buffer.length: 1 [ 14.905084] XXX len: 4096, obj->buffer.length: 1 [ 14.989088] XXX len: 4096, obj->buffer.length: 1 [ 15.073082] XXX len: 4096, obj->buffer.length: 1 [ 15.158086] XXX len: 4096, obj->buffer.length: 1 [ 15.242089] XXX len: 4096, obj->buffer.length: 1 [ 15.326087] XXX len: 4096, obj->buffer.length: 1 [ 15.410085] XXX len: 4096, obj->buffer.length: 1 [ 15.494088] XXX len: 4096, obj->buffer.length: 1 [ 15.578087] XXX len: 4096, obj->buffer.length: 1 [ 15.662086] XXX len: 4096, obj->buffer.length: 1 [ 15.746088] XXX len: 4096, obj->buffer.length: 1 [ 15.830086] XXX len: 4096, obj->buffer.length: 1 [ 15.914084] XXX len: 4096, obj->buffer.length: 1 [ 15.998087] XXX len: 4096, obj->buffer.length: 1 [ 16.082086] XXX len: 4096, obj->buffer.length: 1 [ 16.166086] XXX len: 4096, obj->buffer.length: 1 [ 16.250087] XXX len: 4096, obj->buffer.length: 1 [ 16.334084] XXX len: 4096, obj->buffer.length: 1 [ 16.418086] XXX len: 4096, obj->buffer.length: 1 [ 16.502088] XXX len: 4096, obj->buffer.length: 1 ----------------------------------------- -- Igor