Re: [PATCH] do not try to sync identity map for non-mapped pages
On 04/07/2013 09:34 AM, H. Peter Anvin wrote: > On 04/07/2013 06:33 AM, Borislav Petkov wrote: >> looks like we haven't whacked all the moles - I keep seeing this when >> testing 32-bit builds in qemu on latest Linus + tip. I'd guess this is >> still that /dev/mem accessing thing called wdm. >> >> I'm still wondering though whether we should BUG_ON on a /dev/mem >> access? > > We shouldn't, no. /dev/mem really needs to be fixed along a bunch of > axes. Yes, it is privileged and extra creepy, but it should either work > or it should fail cleanly. I've got a set sitting around collecting dust that I've got to post. It should at least restore sanity for /dev/mem on x86. I'll have it out for some initial review tomorrow. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] do not try to sync identity map for non-mapped pages
On 04/07/2013 09:34 AM, H. Peter Anvin wrote: On 04/07/2013 06:33 AM, Borislav Petkov wrote: looks like we haven't whacked all the moles - I keep seeing this when testing 32-bit builds in qemu on latest Linus + tip. I'd guess this is still that /dev/mem accessing thing called wdm. I'm still wondering though whether we should BUG_ON on a /dev/mem access? We shouldn't, no. /dev/mem really needs to be fixed along a bunch of axes. Yes, it is privileged and extra creepy, but it should either work or it should fail cleanly. I've got a set sitting around collecting dust that I've got to post. It should at least restore sanity for /dev/mem on x86. I'll have it out for some initial review tomorrow. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] do not try to sync identity map for non-mapped pages
On Sun, Apr 07, 2013 at 09:34:07AM -0700, H. Peter Anvin wrote: > We shouldn't, no. /dev/mem really needs to be fixed along a bunch of > axes. Yes, it is privileged and extra creepy, but it should either > work or it should fail cleanly. Can't we filter out accesses through /dev/mem and not BUG_ON in __phys_addr() if they come through /dev/mem? Simply fail cleanly. No idea whether we'll break something still reading /dev/mem though. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] do not try to sync identity map for non-mapped pages
On 04/07/2013 06:33 AM, Borislav Petkov wrote: > > looks like we haven't whacked all the moles - I keep seeing this when > testing 32-bit builds in qemu on latest Linus + tip. I'd guess this is > still that /dev/mem accessing thing called wdm. > > I'm still wondering though whether we should BUG_ON on a /dev/mem > access? > We shouldn't, no. /dev/mem really needs to be fixed along a bunch of axes. Yes, it is privileged and extra creepy, but it should either work or it should fail cleanly. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] do not try to sync identity map for non-mapped pages
Hey Dave, On Thu, Mar 07, 2013 at 08:31:51AM -0800, Dave Hansen wrote: > > The original bug reporter says this fixes it for him, so I'm > broadening the cc list a bit. I assume this should just get > sucked in to the x86 tree. looks like we haven't whacked all the moles - I keep seeing this when testing 32-bit builds in qemu on latest Linus + tip. I'd guess this is still that /dev/mem accessing thing called wdm. I'm still wondering though whether we should BUG_ON on a /dev/mem access? I've added debug output to show why we're triggering: [ 471.102902] slow_virt_to_phys((void *)x): 0x0, phys_addr: 0x37bfe000 [ 471.119500] [ cut here ] [ 471.119500] kernel BUG at arch/x86/mm/physaddr.c:85! [ 471.119500] invalid opcode: [#1] PREEMPT SMP [ 471.119500] Modules linked in: [ 471.119500] Pid: 1571, comm: wdm Not tainted 3.9.0-rc5+ #42 Bochs Bochs [ 471.119500] EIP: 0060:[] EFLAGS: 0206 CPU: 0 [ 471.119500] EIP is at __phys_addr+0x86/0xb0 [ 471.119500] EAX: 37bfe000 EBX: 37bfe000 ECX: 0001 EDX: 37bfe000 [ 471.119500] ESI: f7bfe000 EDI: 2000 EBP: f67f1f3c ESP: f67f1f28 [ 471.119500] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 [ 471.119500] CR0: 8005003b CR2: bfeb12d4 CR3: 35edd000 CR4: 06f0 [ 471.119500] DR0: DR1: DR2: DR3: [ 471.119500] DR6: DR7: [ 471.119500] Process wdm (pid: 1571, ti=f67f task=f5c9 task.ti=f67f) [ 471.119500] Stack: [ 471.119500] c16b1f2c 37bfe000 2000 f67f1f64 c131d074 2000 [ 471.119500] 0246 bfeb12ec f67a6c40 c131d040 2000 f67f1f8c [ 471.119500] c1129c85 f67f1f98 f67f f5ebe864 c131d040 0020 f67a6c40 [ 471.119500] Call Trace: [ 471.119500] [] read_mem+0x34/0x100 [ 471.119500] [] ? write_mem+0x110/0x110 [ 471.119500] [] vfs_read+0x85/0x130 [ 471.119500] [] ? write_mem+0x110/0x110 [ 471.119500] [] sys_read+0x47/0xa0 [ 471.119500] [] sysenter_do_call+0x12/0x36 [ 471.119500] Code: 0b a1 88 ae 0c c2 05 00 00 80 00 39 c6 72 bb a1 ac 1a 76 c1 2d 00 a0 3e 00 25 00 00 e0 ff 2d 00 20 00 00 39 c6 73 a3 0f 0b 0f 0b <0f> 0b 89 f0 e8 41 ca ff ff 89 5c 24 08 89 44 24 04 c7 04 24 2c [ 471.119500] EIP: [] __phys_addr+0x86/0xb0 SS:ESP 0068:f67f1f28 [ 471.508967] ---[ end trace 5fc00ac35d61284a ]--- Hmmm. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] do not try to sync identity map for non-mapped pages
Hey Dave, On Thu, Mar 07, 2013 at 08:31:51AM -0800, Dave Hansen wrote: The original bug reporter says this fixes it for him, so I'm broadening the cc list a bit. I assume this should just get sucked in to the x86 tree. looks like we haven't whacked all the moles - I keep seeing this when testing 32-bit builds in qemu on latest Linus + tip. I'd guess this is still that /dev/mem accessing thing called wdm. I'm still wondering though whether we should BUG_ON on a /dev/mem access? I've added debug output to show why we're triggering: [ 471.102902] slow_virt_to_phys((void *)x): 0x0, phys_addr: 0x37bfe000 [ 471.119500] [ cut here ] [ 471.119500] kernel BUG at arch/x86/mm/physaddr.c:85! [ 471.119500] invalid opcode: [#1] PREEMPT SMP [ 471.119500] Modules linked in: [ 471.119500] Pid: 1571, comm: wdm Not tainted 3.9.0-rc5+ #42 Bochs Bochs [ 471.119500] EIP: 0060:[c1032f56] EFLAGS: 0206 CPU: 0 [ 471.119500] EIP is at __phys_addr+0x86/0xb0 [ 471.119500] EAX: 37bfe000 EBX: 37bfe000 ECX: 0001 EDX: 37bfe000 [ 471.119500] ESI: f7bfe000 EDI: 2000 EBP: f67f1f3c ESP: f67f1f28 [ 471.119500] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 [ 471.119500] CR0: 8005003b CR2: bfeb12d4 CR3: 35edd000 CR4: 06f0 [ 471.119500] DR0: DR1: DR2: DR3: [ 471.119500] DR6: DR7: [ 471.119500] Process wdm (pid: 1571, ti=f67f task=f5c9 task.ti=f67f) [ 471.119500] Stack: [ 471.119500] c16b1f2c 37bfe000 2000 f67f1f64 c131d074 2000 [ 471.119500] 0246 bfeb12ec f67a6c40 c131d040 2000 f67f1f8c [ 471.119500] c1129c85 f67f1f98 f67f f5ebe864 c131d040 0020 f67a6c40 [ 471.119500] Call Trace: [ 471.119500] [c131d074] read_mem+0x34/0x100 [ 471.119500] [c131d040] ? write_mem+0x110/0x110 [ 471.119500] [c1129c85] vfs_read+0x85/0x130 [ 471.119500] [c131d040] ? write_mem+0x110/0x110 [ 471.119500] [c1129e87] sys_read+0x47/0xa0 [ 471.119500] [c1546e5e] sysenter_do_call+0x12/0x36 [ 471.119500] Code: 0b a1 88 ae 0c c2 05 00 00 80 00 39 c6 72 bb a1 ac 1a 76 c1 2d 00 a0 3e 00 25 00 00 e0 ff 2d 00 20 00 00 39 c6 73 a3 0f 0b 0f 0b 0f 0b 89 f0 e8 41 ca ff ff 89 5c 24 08 89 44 24 04 c7 04 24 2c [ 471.119500] EIP: [c1032f56] __phys_addr+0x86/0xb0 SS:ESP 0068:f67f1f28 [ 471.508967] ---[ end trace 5fc00ac35d61284a ]--- Hmmm. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] do not try to sync identity map for non-mapped pages
On 04/07/2013 06:33 AM, Borislav Petkov wrote: looks like we haven't whacked all the moles - I keep seeing this when testing 32-bit builds in qemu on latest Linus + tip. I'd guess this is still that /dev/mem accessing thing called wdm. I'm still wondering though whether we should BUG_ON on a /dev/mem access? We shouldn't, no. /dev/mem really needs to be fixed along a bunch of axes. Yes, it is privileged and extra creepy, but it should either work or it should fail cleanly. -hpa -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] do not try to sync identity map for non-mapped pages
On Sun, Apr 07, 2013 at 09:34:07AM -0700, H. Peter Anvin wrote: We shouldn't, no. /dev/mem really needs to be fixed along a bunch of axes. Yes, it is privileged and extra creepy, but it should either work or it should fail cleanly. Can't we filter out accesses through /dev/mem and not BUG_ON in __phys_addr() if they come through /dev/mem? Simply fail cleanly. No idea whether we'll break something still reading /dev/mem though. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] do not try to sync identity map for non-mapped pages
On 03/07/2013 02:05 PM, Tetsuo Handa wrote: > > Excuse me, but I didn't realize that the link is wrong. > > https://lkml.org/lkml/2013/2/5/396 is a bug in CONFIG_MICROCODE_INTEL_EARLY=y > && CONFIG_64BIT=n && CONFIG_DEBUG_VIRTUAL=y where patches are not available. > > https://lkml.org/lkml/2013/3/5/314 is a bug in CONFIG_ACPI=y && > CONFIG_DEBUG_VIRTUAL=y where your patch fixes. > > Please use https://lkml.org/lkml/2013/3/5/314 rather than > https://lkml.org/lkml/2013/2/5/396 in your patch. > *Grump* I of course already committed it and other things on top. We shouldn't really use lkml.org links anyway since if that site disappears those numbers are meaningless; rather, using Message-IDs (e.g. as embedded in lkml.kernel.org links) is better... -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] do not try to sync identity map for non-mapped pages
Dave Hansen wrote: > > The original bug reporter says this fixes it for him, so I'm > broadening the cc list a bit. I assume this should just get > sucked in to the x86 tree. > > The double-signed-off-by from my is because my IBM email is > going away very shortly. > > -- > > kernel_map_sync_memtype() is called from a variety of contexts. The > pat.c code that calls it seems to ensure that it is not called for > non-ram areas by checking via pat_pagerange_is_ram(). It is important > that it only be called on the actual identity map because there *IS* > no map to sync for highmem pages, or for memory holes. > > The ioremap.c uses are not as careful as those from pat.c, and call > kernel_map_sync_memtype() on PCI space which is in the middle of the > kernel identity map _range_, but is not actually mapped. > > This patch adds a check to kernel_map_sync_memtype() which probably > duplicates some of the checks already in pat.c. But, it is necessary > for the ioremap.c uses and shouldn't hurt other callers. > > I have reproduced this bug and this patch fixes it for me and the > original bug reporter: > > https://lkml.org/lkml/2013/2/5/396 > Excuse me, but I didn't realize that the link is wrong. https://lkml.org/lkml/2013/2/5/396 is a bug in CONFIG_MICROCODE_INTEL_EARLY=y && CONFIG_64BIT=n && CONFIG_DEBUG_VIRTUAL=y where patches are not available. https://lkml.org/lkml/2013/3/5/314 is a bug in CONFIG_ACPI=y && CONFIG_DEBUG_VIRTUAL=y where your patch fixes. Please use https://lkml.org/lkml/2013/3/5/314 rather than https://lkml.org/lkml/2013/2/5/396 in your patch. Regards. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] do not try to sync identity map for non-mapped pages
Dave Hansen wrote: > I have reproduced this bug and this patch fixes it for me > > https://lkml.org/lkml/2013/2/5/396 > > Signed-off-by: Dave Hansen This patch fixes it for me too. Thank you. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] do not try to sync identity map for non-mapped pages
Dave Hansen wrote: I have reproduced this bug and this patch fixes it for me https://lkml.org/lkml/2013/2/5/396 Signed-off-by: Dave Hansen d...@linux.vnet.ibm.com This patch fixes it for me too. Thank you. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] do not try to sync identity map for non-mapped pages
Dave Hansen wrote: The original bug reporter says this fixes it for him, so I'm broadening the cc list a bit. I assume this should just get sucked in to the x86 tree. The double-signed-off-by from my is because my IBM email is going away very shortly. -- kernel_map_sync_memtype() is called from a variety of contexts. The pat.c code that calls it seems to ensure that it is not called for non-ram areas by checking via pat_pagerange_is_ram(). It is important that it only be called on the actual identity map because there *IS* no map to sync for highmem pages, or for memory holes. The ioremap.c uses are not as careful as those from pat.c, and call kernel_map_sync_memtype() on PCI space which is in the middle of the kernel identity map _range_, but is not actually mapped. This patch adds a check to kernel_map_sync_memtype() which probably duplicates some of the checks already in pat.c. But, it is necessary for the ioremap.c uses and shouldn't hurt other callers. I have reproduced this bug and this patch fixes it for me and the original bug reporter: https://lkml.org/lkml/2013/2/5/396 Excuse me, but I didn't realize that the link is wrong. https://lkml.org/lkml/2013/2/5/396 is a bug in CONFIG_MICROCODE_INTEL_EARLY=y CONFIG_64BIT=n CONFIG_DEBUG_VIRTUAL=y where patches are not available. https://lkml.org/lkml/2013/3/5/314 is a bug in CONFIG_ACPI=y CONFIG_DEBUG_VIRTUAL=y where your patch fixes. Please use https://lkml.org/lkml/2013/3/5/314 rather than https://lkml.org/lkml/2013/2/5/396 in your patch. Regards. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] do not try to sync identity map for non-mapped pages
On 03/07/2013 02:05 PM, Tetsuo Handa wrote: Excuse me, but I didn't realize that the link is wrong. https://lkml.org/lkml/2013/2/5/396 is a bug in CONFIG_MICROCODE_INTEL_EARLY=y CONFIG_64BIT=n CONFIG_DEBUG_VIRTUAL=y where patches are not available. https://lkml.org/lkml/2013/3/5/314 is a bug in CONFIG_ACPI=y CONFIG_DEBUG_VIRTUAL=y where your patch fixes. Please use https://lkml.org/lkml/2013/3/5/314 rather than https://lkml.org/lkml/2013/2/5/396 in your patch. *Grump* I of course already committed it and other things on top. We shouldn't really use lkml.org links anyway since if that site disappears those numbers are meaningless; rather, using Message-IDs (e.g. as embedded in lkml.kernel.org links) is better... -hpa -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
