Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1591447 > unrolled thread
| Started by | Fengguang Wu <fengguang.wu@intel.com> |
|---|---|
| First post | 2017-03-02 21:30 +0100 |
| Last post | 2017-03-09 14:50 +0100 |
| Articles | 20 on this page of 30 — 8 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Fengguang Wu <fengguang.wu@intel.com> - 2017-03-02 21:30 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Daniel Borkmann <daniel@iogearbox.net> - 2017-03-02 22:40 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-08 20:30 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Kees Cook <keescook@chromium.org> - 2017-03-08 23:40 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Daniel Borkmann <daniel@iogearbox.net> - 2017-03-09 00:20 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Laura Abbott <labbott@redhat.com> - 2017-03-09 01:30 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Kees Cook <keescook@chromium.org> - 2017-03-09 06:40 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Daniel Borkmann <daniel@iogearbox.net> - 2017-03-09 14:10 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Thomas Gleixner <tglx@linutronix.de> - 2017-03-09 14:20 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Daniel Borkmann <daniel@iogearbox.net> - 2017-03-09 15:10 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Thomas Gleixner <tglx@linutronix.de> - 2017-03-09 16:00 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Daniel Borkmann <daniel@iogearbox.net> - 2017-03-09 19:00 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf David Miller <davem@davemloft.net> - 2017-03-09 19:10 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-09 19:20 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-09 19:20 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Daniel Borkmann <daniel@iogearbox.net> - 2017-03-09 19:40 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Daniel Borkmann <daniel@iogearbox.net> - 2017-03-09 22:40 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Borislav Petkov <bp@suse.de> - 2017-03-09 23:10 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Daniel Borkmann <daniel@iogearbox.net> - 2017-03-09 23:20 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Borislav Petkov <bp@suse.de> - 2017-03-09 23:50 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-10 00:30 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Borislav Petkov <bp@suse.de> - 2017-03-10 00:50 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Daniel Borkmann <daniel@iogearbox.net> - 2017-03-10 01:20 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Borislav Petkov <bp@suse.de> - 2017-03-12 22:50 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Borislav Petkov <bp@suse.de> - 2017-03-09 23:20 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Daniel Borkmann <daniel@iogearbox.net> - 2017-03-09 16:00 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-09 18:50 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-08 23:50 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Fengguang Wu <fengguang.wu@intel.com> - 2017-03-09 02:40 +0100
Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf Thomas Gleixner <tglx@linutronix.de> - 2017-03-09 14:50 +0100
Page 1 of 2 [1] 2 Next page →
| From | Fengguang Wu <fengguang.wu@intel.com> |
|---|---|
| Date | 2017-03-02 21:30 +0100 |
| Subject | Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf |
| Message-ID | <tgFwm-7r6-11@gated-at.bofh.it> |
On Wed, Mar 01, 2017 at 08:54:26PM +0800, Fengguang Wu wrote: >Hi all, > >Is it BPF triggering BUGs all over the places? It looks so, and here is a fix. >1e74a2eb1f Merge tag 'gcc-plugins-v4.11-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux >005c3490e9 Revert "ath10k: Search SMBIOS for OEM board file extension" >3051bf36c2 Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next >+-------------------------------------------------------+------------+------------+------------+ >| | 1e74a2eb1f | 005c3490e9 | 3051bf36c2 | >+-------------------------------------------------------+------------+------------+------------+ >| boot_successes | 1223 | 1098 | 242 | >| boot_failures | 1 | 126 | 72 | >| BUG:unable_to_handle_kernel | 1 | 117 | 69 | >| Oops | 1 | 126 | 72 | >| EIP:perf_callchain_user | 1 | | | >| Kernel_panic-not_syncing:Fatal_exception | 1 | 121 | 67 | >| EIP:netlink_release | 0 | 20 | 3 | >| EIP:bpf_prog_free | 0 | 22 | 3 | >| EIP:filp_close | 0 | 64 | 23 | >| EIP:netlink_update_listeners | 0 | 10 | 9 | >| EIP:security_inode_getattr | 0 | 2 | | >| EIP:__lock_acquire | 0 | 1 | 11 | >| Kernel_panic-not_syncing:Fatal_exception_in_interrupt | 0 | 5 | 4 | >| EIP:__rcu_process_callbacks | 0 | 2 | | >| EIP:__fget_light | 0 | 1 | | >| EIP:__unix_remove_socket | 0 | 0 | 13 | >| INFO:trying_to_register_non-static_key | 0 | 0 | 2 | >| EIP:mnt_want_write_file | 0 | 0 | 1 | >| EIP:skb_dequeue | 0 | 0 | 1 | >| EIP:strlen | 0 | 0 | 1 | >| EIP:__netlink_lookup | 0 | 0 | 2 | >| EIP:vfs_fsync_range | 0 | 0 | 1 | >| EIP:__unix_find_socket_byname | 0 | 0 | 1 | >| EIP:release_sock | 0 | 0 | 1 | >+-------------------------------------------------------+------------+------------+------------+ I confirm that the below patch provided by Daniel fixes the above issues on mainline kernel, too. Where should this patch be sent to? It'd be very noisy if all these Oops hit the upcoming RC1 kernel. Daniel thinks there may be deeper problem in i386 set_memory_rw(). However that could take much longer time to debug. Thanks, Fengguang --- Re: [bpf] 9d876e79df: BUG: unable to handle kernel paging request at 653a8346 > On Tue, Feb 28, 2017 at 04:39:36PM +0100, Daniel Borkmann wrote: I have a rough feeling what it is, but I didn't have cycles to work on it yet (due to travel, sorry about that). The issue is likely shut down by just doing: --- arch/x86/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- linux.orig/arch/x86/Kconfig 2017-03-03 03:44:35.962022996 +0800 +++ linux/arch/x86/Kconfig 2017-03-03 03:44:35.962022996 +0800 @@ -54,7 +54,7 @@ config X86 select ARCH_HAS_KCOV if X86_64 select ARCH_HAS_MMIO_FLUSH select ARCH_HAS_PMEM_API if X86_64 - select ARCH_HAS_SET_MEMORY + select ARCH_HAS_SET_MEMORY if X86_64 select ARCH_HAS_SG_CHAIN select ARCH_HAS_STRICT_KERNEL_RWX select ARCH_HAS_STRICT_MODULE_RWX
[toc] | [next] | [standalone]
| From | Daniel Borkmann <daniel@iogearbox.net> |
|---|---|
| Date | 2017-03-02 22:40 +0100 |
| Message-ID | <tgGC5-885-3@gated-at.bofh.it> |
| In reply to | #1591447 |
On 03/02/2017 09:23 PM, Fengguang Wu wrote: [...] > I confirm that the below patch provided by Daniel fixes the above > issues on mainline kernel, too. Where should this patch be sent to? If nobody objects, I could send it to -net tree via Dave due to being BPF related, but I don't mind sending it elsewhere too (f.e. Linus directly?) in order to stop your bot from continuing to send such mails. The issue seems only related to i386 and doesn't trigger each time with Fengguang's kernel config and qemu image when I try to reproduce it. set_memory_ro()/set_memory_rw() on i386 seems to work in general, but when it's used/reproduced, from time to time (perhaps some corner-case?) it looks like that memory area can have issues much later on after being fed back to the allocator which then causes a GPF from random locations. Gut feeling, it might be an issue in set_memory_*() that my commit uncovered. Still looking into it, but mean-time I could just send the below, sure. Thanks, Daniel > It'd be very noisy if all these Oops hit the upcoming RC1 kernel. > > Daniel thinks there may be deeper problem in i386 set_memory_rw(). > However that could take much longer time to debug. > > Thanks, > Fengguang > --- > > Re: [bpf] 9d876e79df: BUG: unable to handle kernel paging request at 653a8346 > >> On Tue, Feb 28, 2017 at 04:39:36PM +0100, Daniel Borkmann wrote: > > I have a rough feeling what it is, but I didn't have cycles to work on > it yet (due to travel, sorry about that). The issue is likely shut down > by just doing: > > --- > arch/x86/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > --- linux.orig/arch/x86/Kconfig 2017-03-03 03:44:35.962022996 +0800 > +++ linux/arch/x86/Kconfig 2017-03-03 03:44:35.962022996 +0800 > @@ -54,7 +54,7 @@ config X86 > select ARCH_HAS_KCOV if X86_64 > select ARCH_HAS_MMIO_FLUSH > select ARCH_HAS_PMEM_API if X86_64 > - select ARCH_HAS_SET_MEMORY > + select ARCH_HAS_SET_MEMORY if X86_64 > select ARCH_HAS_SG_CHAIN > select ARCH_HAS_STRICT_KERNEL_RWX > select ARCH_HAS_STRICT_MODULE_RWX
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2017-03-08 20:30 +0100 |
| Message-ID | <tiPrA-2uR-1@gated-at.bofh.it> |
| In reply to | #1591481 |
Adding x86 people too, since this seems to be something off about
ARCH_HAS_SET_MEMORY for x86-32.
The code seems to be shared between x86-32 and 64, I'm not seeing why
set_memory_r[ow]() should fail on one but not the other.
Considering that it seems to be flaky even on 32-bit, maybe it's
timing-related, or possibly related to TLB sizes or whatever (ie more
likely hidden by a larger TLB on more modern hardware?)
Anyway, just looking at change_page_attr_set_clr(), I notice that the
page alias checking treats NX specially:
/* No alias checking for _NX bit modifications */
checkalias = (pgprot_val(mask_set) | pgprot_val(mask_clr)) != _PAGE_NX;
which seems insane. Why would NX be different from other protection
bits (like _PAGE_RW)?
But that doesn't explain why the bpf code would have issues with this
all only on x86-32.
Maybe somebody else can see why ARCH_HAS_SET_MEMORY would depend on
64-bit only..
Linus
On Thu, Mar 2, 2017 at 12:40 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 03/02/2017 09:23 PM, Fengguang Wu wrote:
> [...]
>>
>> I confirm that the below patch provided by Daniel fixes the above
>> issues on mainline kernel, too. Where should this patch be sent to?
>
>
> If nobody objects, I could send it to -net tree via Dave due to being
> BPF related, but I don't mind sending it elsewhere too (f.e. Linus
> directly?) in order to stop your bot from continuing to send such mails.
>
> The issue seems only related to i386 and doesn't trigger each time with
> Fengguang's kernel config and qemu image when I try to reproduce it.
> set_memory_ro()/set_memory_rw() on i386 seems to work in general, but
> when it's used/reproduced, from time to time (perhaps some corner-case?)
> it looks like that memory area can have issues much later on after being
> fed back to the allocator which then causes a GPF from random locations.
> Gut feeling, it might be an issue in set_memory_*() that my commit
> uncovered. Still looking into it, but mean-time I could just send the
> below, sure.
>
> Thanks,
> Daniel
>
>
>> It'd be very noisy if all these Oops hit the upcoming RC1 kernel.
>>
>> Daniel thinks there may be deeper problem in i386 set_memory_rw().
>> However that could take much longer time to debug.
>>
>> Thanks,
>> Fengguang
>> ---
>>
>> Re: [bpf] 9d876e79df: BUG: unable to handle kernel paging request at
>> 653a8346
>>
>>> On Tue, Feb 28, 2017 at 04:39:36PM +0100, Daniel Borkmann wrote:
>>
>>
>> I have a rough feeling what it is, but I didn't have cycles to work on
>> it yet (due to travel, sorry about that). The issue is likely shut down
>> by just doing:
>>
>> ---
>> arch/x86/Kconfig | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> --- linux.orig/arch/x86/Kconfig 2017-03-03 03:44:35.962022996 +0800
>> +++ linux/arch/x86/Kconfig 2017-03-03 03:44:35.962022996 +0800
>> @@ -54,7 +54,7 @@ config X86
>> select ARCH_HAS_KCOV if X86_64
>> select ARCH_HAS_MMIO_FLUSH
>> select ARCH_HAS_PMEM_API if X86_64
>> - select ARCH_HAS_SET_MEMORY
>> + select ARCH_HAS_SET_MEMORY if X86_64
>> select ARCH_HAS_SG_CHAIN
>> select ARCH_HAS_STRICT_KERNEL_RWX
>> select ARCH_HAS_STRICT_MODULE_RWX
>
>
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-03-08 23:40 +0100 |
| Message-ID | <tiSps-4tU-13@gated-at.bofh.it> |
| In reply to | #1595460 |
On Wed, Mar 8, 2017 at 2:27 PM, Daniel Borkmann <daniel@iogearbox.net> wrote: > [ 28.474232] rodata_test: test data was not read only > [...] In my tests so far, I've never been able to get rodata_test to fail (Qemu 2.5.0, Ubuntu). I'll retry with your .config and see if I can recheck under Qemu 2.7.1. Do you see these failures on real hardware? -Kees -- Kees Cook Pixel Security
[toc] | [prev] | [next] | [standalone]
| From | Daniel Borkmann <daniel@iogearbox.net> |
|---|---|
| Date | 2017-03-09 00:20 +0100 |
| Message-ID | <tiT2a-4ZD-15@gated-at.bofh.it> |
| In reply to | #1595561 |
On 03/08/2017 11:36 PM, Kees Cook wrote: > On Wed, Mar 8, 2017 at 2:27 PM, Daniel Borkmann <daniel@iogearbox.net> wrote: >> [ 28.474232] rodata_test: test data was not read only >> [...] > > In my tests so far, I've never been able to get rodata_test to fail > (Qemu 2.5.0, Ubuntu). I'll retry with your .config and see if I can > recheck under Qemu 2.7.1. Do you see these failures on real hardware? The x86_64 tests on real hardware, and the x86-32 only in the qemu environment with the reproducer script. Haven't tested x86-32 kernel outside of qemu so far. Thanks, Daniel
[toc] | [prev] | [next] | [standalone]
| From | Laura Abbott <labbott@redhat.com> |
|---|---|
| Date | 2017-03-09 01:30 +0100 |
| Message-ID | <tiU7T-5GA-5@gated-at.bofh.it> |
| In reply to | #1595561 |
On 03/08/2017 02:36 PM, Kees Cook wrote: > On Wed, Mar 8, 2017 at 2:27 PM, Daniel Borkmann <daniel@iogearbox.net> wrote: >> [ 28.474232] rodata_test: test data was not read only >> [...] > > In my tests so far, I've never been able to get rodata_test to fail > (Qemu 2.5.0, Ubuntu). I'll retry with your .config and see if I can > recheck under Qemu 2.7.1. Do you see these failures on real hardware? > > -Kees > FWIW, I'm seeing the same issue with qemu 2.6.2 and 2.8.0 on Fedora 24 and rawhide respectively. I also notice that CONFIG_X86_PAE is turned off in the defconfig. If I set CONFIG_HIGHMEM_64G which turns on CONFIG_X86_PAE the problem goes away. I can't tell if this is an indication of magically hiding the TLB problem or if there is an issue with !X86_PAE invalidation. Thanks, Laura
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-03-09 06:40 +0100 |
| Message-ID | <tiYXT-xV-1@gated-at.bofh.it> |
| In reply to | #1595599 |
On Wed, Mar 8, 2017 at 3:55 PM, Laura Abbott <labbott@redhat.com> wrote: > On 03/08/2017 02:36 PM, Kees Cook wrote: >> On Wed, Mar 8, 2017 at 2:27 PM, Daniel Borkmann <daniel@iogearbox.net> wrote: >>> [ 28.474232] rodata_test: test data was not read only >>> [...] >> >> In my tests so far, I've never been able to get rodata_test to fail >> (Qemu 2.5.0, Ubuntu). I'll retry with your .config and see if I can >> recheck under Qemu 2.7.1. Do you see these failures on real hardware? >> >> -Kees >> > > FWIW, I'm seeing the same issue with qemu 2.6.2 and 2.8.0 on Fedora 24 > and rawhide respectively. > > I also notice that CONFIG_X86_PAE is turned off in the defconfig. If > I set CONFIG_HIGHMEM_64G which turns on CONFIG_X86_PAE the problem > goes away. I can't tell if this is an indication of magically hiding > the TLB problem or if there is an issue with !X86_PAE invalidation. I found my difference. I normally run qemu with "-cpu host" which makes the failure go away. With "-cpu kvm64", I see the rodata_test failure immediately. Seems like this may be a kvm cpu feature emulation bug? I'll see if I can find the specific cpu feature in the morning... -Kees -- Kees Cook Pixel Security
[toc] | [prev] | [next] | [standalone]
| From | Daniel Borkmann <daniel@iogearbox.net> |
|---|---|
| Date | 2017-03-09 14:10 +0100 |
| Message-ID | <tj5Zp-5Ce-49@gated-at.bofh.it> |
| In reply to | #1595679 |
On 03/09/2017 06:36 AM, Kees Cook wrote: > On Wed, Mar 8, 2017 at 3:55 PM, Laura Abbott <labbott@redhat.com> wrote: >> On 03/08/2017 02:36 PM, Kees Cook wrote: >>> On Wed, Mar 8, 2017 at 2:27 PM, Daniel Borkmann <daniel@iogearbox.net> wrote: >>>> [ 28.474232] rodata_test: test data was not read only >>>> [...] >>> >>> In my tests so far, I've never been able to get rodata_test to fail >>> (Qemu 2.5.0, Ubuntu). I'll retry with your .config and see if I can >>> recheck under Qemu 2.7.1. Do you see these failures on real hardware? >>> >>> -Kees >> >> FWIW, I'm seeing the same issue with qemu 2.6.2 and 2.8.0 on Fedora 24 >> and rawhide respectively. >> >> I also notice that CONFIG_X86_PAE is turned off in the defconfig. If >> I set CONFIG_HIGHMEM_64G which turns on CONFIG_X86_PAE the problem >> goes away. I can't tell if this is an indication of magically hiding >> the TLB problem or if there is an issue with !X86_PAE invalidation. > > I found my difference. I normally run qemu with "-cpu host" which > makes the failure go away. With "-cpu kvm64", I see the rodata_test > failure immediately. Seems like this may be a kvm cpu feature > emulation bug? I'll see if I can find the specific cpu feature in the > morning... Interesting! Changing to "-cpu host" makes rodata_test succeed plus my test_setmem and the test_bpf suite runs fine as well. Haven't seen a corruption since. Switching back to "-cpu kvm64" I immediately see mentioned issues again. With regard to CPA_FLUSHTLB that Linus mentioned, when I investigated code paths in change_page_attr_set_clr(), I did see that CPA_FLUSHTLB was set each time we switched attrs and a cpa_flush_range() was performed (with the correct number of pages and cache set to 0). That would be a __flush_tlb_all() eventually. Hmm, it indeed might seem likely that this could be an emulation bug. Thanks, Daniel
[toc] | [prev] | [next] | [standalone]
| From | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| Date | 2017-03-09 14:20 +0100 |
| Message-ID | <tj693-5FN-5@gated-at.bofh.it> |
| In reply to | #1596054 |
On Thu, 9 Mar 2017, Daniel Borkmann wrote: > With regard to CPA_FLUSHTLB that Linus mentioned, when I investigated > code paths in change_page_attr_set_clr(), I did see that CPA_FLUSHTLB > was set each time we switched attrs and a cpa_flush_range() was > performed (with the correct number of pages and cache set to 0). That > would be a __flush_tlb_all() eventually. > > Hmm, it indeed might seem likely that this could be an emulation bug. Which variant of __flush_tlb_all() is used when the test fails? Check for the following flags in /proc/cpuinfo: pge invpcid Thanks, tglx
[toc] | [prev] | [next] | [standalone]
| From | Daniel Borkmann <daniel@iogearbox.net> |
|---|---|
| Date | 2017-03-09 15:10 +0100 |
| Message-ID | <tj6Vr-6eP-3@gated-at.bofh.it> |
| In reply to | #1596056 |
On 03/09/2017 02:10 PM, Thomas Gleixner wrote:
> On Thu, 9 Mar 2017, Daniel Borkmann wrote:
>> With regard to CPA_FLUSHTLB that Linus mentioned, when I investigated
>> code paths in change_page_attr_set_clr(), I did see that CPA_FLUSHTLB
>> was set each time we switched attrs and a cpa_flush_range() was
>> performed (with the correct number of pages and cache set to 0). That
>> would be a __flush_tlb_all() eventually.
>>
>> Hmm, it indeed might seem likely that this could be an emulation bug.
>
> Which variant of __flush_tlb_all() is used when the test fails?
>
> Check for the following flags in /proc/cpuinfo: pge invpcid
I added the following and booted with both variants:
printk("X86_FEATURE_PGE:%u\n", static_cpu_has(X86_FEATURE_PGE));
printk("X86_FEATURE_INVPCID:%u\n", static_cpu_has(X86_FEATURE_INVPCID));
"-cpu host" gives:
[ 8.326117] X86_FEATURE_PGE:1
[ 8.326381] X86_FEATURE_INVPCID:1
"-cpu kvm64" gives:
[ 8.517069] X86_FEATURE_PGE:1
[ 8.517393] X86_FEATURE_INVPCID:0
Thanks,
Daniel
[toc] | [prev] | [next] | [standalone]
| From | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| Date | 2017-03-09 16:00 +0100 |
| Message-ID | <tj7HQ-6AN-9@gated-at.bofh.it> |
| In reply to | #1596081 |
On Thu, 9 Mar 2017, Daniel Borkmann wrote:
> On 03/09/2017 02:10 PM, Thomas Gleixner wrote:
> > On Thu, 9 Mar 2017, Daniel Borkmann wrote:
> > > With regard to CPA_FLUSHTLB that Linus mentioned, when I investigated
> > > code paths in change_page_attr_set_clr(), I did see that CPA_FLUSHTLB
> > > was set each time we switched attrs and a cpa_flush_range() was
> > > performed (with the correct number of pages and cache set to 0). That
> > > would be a __flush_tlb_all() eventually.
> > >
> > > Hmm, it indeed might seem likely that this could be an emulation bug.
> >
> > Which variant of __flush_tlb_all() is used when the test fails?
> >
> > Check for the following flags in /proc/cpuinfo: pge invpcid
>
> I added the following and booted with both variants:
>
> printk("X86_FEATURE_PGE:%u\n", static_cpu_has(X86_FEATURE_PGE));
> printk("X86_FEATURE_INVPCID:%u\n", static_cpu_has(X86_FEATURE_INVPCID));
>
> "-cpu host" gives:
>
> [ 8.326117] X86_FEATURE_PGE:1
> [ 8.326381] X86_FEATURE_INVPCID:1
>
> "-cpu kvm64" gives:
>
> [ 8.517069] X86_FEATURE_PGE:1
> [ 8.517393] X86_FEATURE_INVPCID:0
That's the one which fails. So it's using the CR4 based flushing. Just ran
a test on a physical system with PGE=1 and INVPCID=0. Works fine.
Emulation problem?
Thanks,
tglx
[toc] | [prev] | [next] | [standalone]
| From | Daniel Borkmann <daniel@iogearbox.net> |
|---|---|
| Date | 2017-03-09 19:00 +0100 |
| Message-ID | <tjaw2-8vb-3@gated-at.bofh.it> |
| In reply to | #1596127 |
On 03/09/2017 03:49 PM, Thomas Gleixner wrote:
> On Thu, 9 Mar 2017, Daniel Borkmann wrote:
>> On 03/09/2017 02:10 PM, Thomas Gleixner wrote:
>>> On Thu, 9 Mar 2017, Daniel Borkmann wrote:
>>>> With regard to CPA_FLUSHTLB that Linus mentioned, when I investigated
>>>> code paths in change_page_attr_set_clr(), I did see that CPA_FLUSHTLB
>>>> was set each time we switched attrs and a cpa_flush_range() was
>>>> performed (with the correct number of pages and cache set to 0). That
>>>> would be a __flush_tlb_all() eventually.
>>>>
>>>> Hmm, it indeed might seem likely that this could be an emulation bug.
>>>
>>> Which variant of __flush_tlb_all() is used when the test fails?
>>>
>>> Check for the following flags in /proc/cpuinfo: pge invpcid
>>
>> I added the following and booted with both variants:
>>
>> printk("X86_FEATURE_PGE:%u\n", static_cpu_has(X86_FEATURE_PGE));
>> printk("X86_FEATURE_INVPCID:%u\n", static_cpu_has(X86_FEATURE_INVPCID));
>>
>> "-cpu host" gives:
>>
>> [ 8.326117] X86_FEATURE_PGE:1
>> [ 8.326381] X86_FEATURE_INVPCID:1
>>
>> "-cpu kvm64" gives:
>>
>> [ 8.517069] X86_FEATURE_PGE:1
>> [ 8.517393] X86_FEATURE_INVPCID:0
>
> That's the one which fails. So it's using the CR4 based flushing. Just ran
> a test on a physical system with PGE=1 and INVPCID=0. Works fine.
>
> Emulation problem?
So in the git qemu code base (target/i386/helper.c), cr3 vs cr4 looks
like the following, both sharing the tlb_flush() itself:
void cpu_x86_update_cr3(CPUX86State *env, target_ulong new_cr3)
{
X86CPU *cpu = x86_env_get_cpu(env);
env->cr[3] = new_cr3;
if (env->cr[0] & CR0_PG_MASK) {
qemu_log_mask(CPU_LOG_MMU,
"CR3 update: CR3=" TARGET_FMT_lx "\n", new_cr3);
tlb_flush(CPU(cpu));
}
}
void cpu_x86_update_cr4(CPUX86State *env, uint32_t new_cr4)
{
X86CPU *cpu = x86_env_get_cpu(env);
uint32_t hflags;
#if defined(DEBUG_MMU)
printf("CR4 update: %08x -> %08x\n", (uint32_t)env->cr[4], new_cr4);
#endif
if ((new_cr4 ^ env->cr[4]) &
(CR4_PGE_MASK | CR4_PAE_MASK | CR4_PSE_MASK |
CR4_SMEP_MASK | CR4_SMAP_MASK | CR4_LA57_MASK)) {
tlb_flush(CPU(cpu));
}
[...]
}
I added some debugging around __native_flush_tlb_global_irq_disabled()
and if I understand it correctly, the idea of cr4 is that we need to
toggle X86_CR4_PGE in order to trigger a TLB flush.
What I see is that original cr4 is 0x610. The cpu_tlbstate.cr4 is
consistent to native_read_cr4() and since cr4 is != 0, it tells me
based on the comment in native_read_cr4() that cr4 seems to be
supported. Thus, meaning we end up with writing ...
native_write_cr4(0x610);
native_write_cr4(0x610);
... twice, and this just doesn't trigger the desired TLB flush. I
changed the code into the following ...
cr4 = this_cpu_read(cpu_tlbstate.cr4);
/* clear PGE */
- native_write_cr4(cr4 & ~X86_CR4_PGE);
+ native_write_cr4(cr4 ^ X86_CR4_PGE);
/* write old PGE again and flush TLBs */
native_write_cr4(cr4);
... and the test cases seem to be working for me now with "-cpu kvm64",
so that seems to trigger the TLB we were missing.
I don't know enough about x86 internals to tell whether the change is
sane, though, but it seems at least for qemu fwiw. ;) Thoughts?
Thanks,
Daniel
[toc] | [prev] | [next] | [standalone]
| From | David Miller <davem@davemloft.net> |
|---|---|
| Date | 2017-03-09 19:10 +0100 |
| Subject | Re: [net/bpf] 3051bf36c2 BUG: unable to handle kernel paging request at 0000a7cf |
| Message-ID | <tjaFI-ma-33@gated-at.bofh.it> |
| In reply to | #1596262 |
From: Daniel Borkmann <daniel@iogearbox.net> Date: Thu, 09 Mar 2017 18:51:03 +0100 > I added some debugging around __native_flush_tlb_global_irq_disabled() > and if I understand it correctly, the idea of cr4 is that we need to > toggle X86_CR4_PGE in order to trigger a TLB flush. > > What I see is that original cr4 is 0x610. The cpu_tlbstate.cr4 is > consistent to native_read_cr4() and since cr4 is != 0, it tells me > based on the comment in native_read_cr4() that cr4 seems to be > supported. Thus, meaning we end up with writing ... > > native_write_cr4(0x610); > native_write_cr4(0x610); > > ... twice, and this just doesn't trigger the desired TLB flush. I > changed the code into the following ... > > cr4 = this_cpu_read(cpu_tlbstate.cr4); > /* clear PGE */ > - native_write_cr4(cr4 & ~X86_CR4_PGE); > + native_write_cr4(cr4 ^ X86_CR4_PGE); > /* write old PGE again and flush TLBs */ > native_write_cr4(cr4); > > ... and the test cases seem to be working for me now with "-cpu > kvm64", > so that seems to trigger the TLB we were missing. Great detective work Daniel.
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2017-03-09 19:20 +0100 |
| Message-ID | <tjaPo-rT-23@gated-at.bofh.it> |
| In reply to | #1596262 |
On Thu, Mar 9, 2017 at 9:51 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> What I see is that original cr4 is 0x610. The cpu_tlbstate.cr4 is
> consistent to native_read_cr4() and since cr4 is != 0, it tells me
> based on the comment in native_read_cr4() that cr4 seems to be
> supported. Thus, meaning we end up with writing ...
>
> native_write_cr4(0x610);
> native_write_cr4(0x610);
>
> ... twice, and this just doesn't trigger the desired TLB flush.
Very odd. We should always have PGE (0x0080) set in cr4 (if the CPU
supports it).
But yes, if PGE is clear then that certainly explains the bug, and
it's not an emulation issue.
> I changed the code into the following ...
>
> cr4 = this_cpu_read(cpu_tlbstate.cr4);
> /* clear PGE */
> - native_write_cr4(cr4 & ~X86_CR4_PGE);
> + native_write_cr4(cr4 ^ X86_CR4_PGE);
> /* write old PGE again and flush TLBs */
> native_write_cr4(cr4);
Yeah, good for debugging, but not a good patch in general. The only
valid reason for not having PGE enabled would be that the CPU doesn't
support PGE at all.
Linus
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2017-03-09 19:20 +0100 |
| Message-ID | <tjaPo-rT-25@gated-at.bofh.it> |
| In reply to | #1596289 |
On Thu, Mar 9, 2017 at 10:10 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Very odd. We should always have PGE (0x0080) set in cr4 (if the CPU
> supports it).
Daniel, do you see the code in probe_page_size_mask() triggering?
/* Enable PGE if available */
if (boot_cpu_has(X86_FEATURE_PGE)) {
cr4_set_bits_and_update_boot(X86_CR4_PGE);
__supported_pte_mask |= _PAGE_GLOBAL;
} else
__supported_pte_mask &= ~_PAGE_GLOBAL;
but maybe there's something wrong with the percpu cr4 caching?
Linus
[toc] | [prev] | [next] | [standalone]
| From | Daniel Borkmann <daniel@iogearbox.net> |
|---|---|
| Date | 2017-03-09 19:40 +0100 |
| Message-ID | <tjb8J-yF-1@gated-at.bofh.it> |
| In reply to | #1596291 |
On 03/09/2017 07:15 PM, Linus Torvalds wrote:
> On Thu, Mar 9, 2017 at 10:10 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> Very odd. We should always have PGE (0x0080) set in cr4 (if the CPU
>> supports it).
>
> Daniel, do you see the code in probe_page_size_mask() triggering?
>
> /* Enable PGE if available */
> if (boot_cpu_has(X86_FEATURE_PGE)) {
> cr4_set_bits_and_update_boot(X86_CR4_PGE);
> __supported_pte_mask |= _PAGE_GLOBAL;
We do have boot_cpu_has(X86_FEATURE_PGE) and go indeed into this
branch here. So it seems something must be clearing it later, hmm.
> } else
> __supported_pte_mask &= ~_PAGE_GLOBAL;
>
> but maybe there's something wrong with the percpu cr4 caching?
>
> Linus
>
[toc] | [prev] | [next] | [standalone]
| From | Daniel Borkmann <daniel@iogearbox.net> |
|---|---|
| Date | 2017-03-09 22:40 +0100 |
| Message-ID | <tjdWV-2rg-9@gated-at.bofh.it> |
| In reply to | #1596301 |
[ + Borislav ]
On 03/09/2017 07:31 PM, Daniel Borkmann wrote:
> On 03/09/2017 07:15 PM, Linus Torvalds wrote:
>> On Thu, Mar 9, 2017 at 10:10 AM, Linus Torvalds
>> <torvalds@linux-foundation.org> wrote:
>>>
>>> Very odd. We should always have PGE (0x0080) set in cr4 (if the CPU
>>> supports it).
>>
>> Daniel, do you see the code in probe_page_size_mask() triggering?
>>
>> /* Enable PGE if available */
>> if (boot_cpu_has(X86_FEATURE_PGE)) {
>> cr4_set_bits_and_update_boot(X86_CR4_PGE);
>> __supported_pte_mask |= _PAGE_GLOBAL;
>
> We do have boot_cpu_has(X86_FEATURE_PGE) and go indeed into this
> branch here. So it seems something must be clearing it later, hmm.
>
>> } else
>> __supported_pte_mask &= ~_PAGE_GLOBAL;
>>
>> but maybe there's something wrong with the percpu cr4 caching?
So, I think I got a little bit further. To the printk's I added
previously in the test_setmem code that run on the problematic
kernel, I now extended them into:
printk("static_cpu X86_FEATURE_PGE:%u\n", static_cpu_has(X86_FEATURE_PGE));
printk("boot_cpu X86_FEATURE_PGE:%u\n", boot_cpu_has(X86_FEATURE_PGE));
printk("static_cpu X86_FEATURE_INVPCID:%u\n", static_cpu_has(X86_FEATURE_INVPCID));
printk("boot_cpu X86_FEATURE_INVPCID:%u\n", boot_cpu_has(X86_FEATURE_INVPCID));
And here's what I get in the log:
"-cpu kvm64" gives:
[ 8.426865] static_cpu X86_FEATURE_PGE:1
[ 8.427148] boot_cpu X86_FEATURE_PGE:0
[ 8.427428] static_cpu X86_FEATURE_INVPCID:0
[ 8.427732] boot_cpu X86_FEATURE_INVPCID:0
"-cpu host" gives:
[ 8.426408] static_cpu X86_FEATURE_PGE:1
[ 8.426726] boot_cpu X86_FEATURE_PGE:0
[ 8.427037] static_cpu X86_FEATURE_INVPCID:1
[ 8.427375] boot_cpu X86_FEATURE_INVPCID:1
This means at that point in time static_cpu_has(X86_FEATURE_PGE) is
not the same as boot_cpu_has(X86_FEATURE_PGE).
The code that switches this off is in lguest_arch_host_init(). Right
before that, both are X86_FEATURE_PGE:1, X86_FEATURE_INVPCID:0 for
the "-cpu kvm64" case.
Then, the lguest code does:
get_online_cpus();
if (boot_cpu_has(X86_FEATURE_PGE)) { /* We have a broader idea of "global". */
/* Remember that this was originally set (for cleanup). */
cpu_had_pge = 1;
/*
* adjust_pge is a helper function which sets or unsets the PGE
* bit on its CPU, depending on the argument (0 == unset).
*/
on_each_cpu(adjust_pge, (void *)0, 1);
/* Turn off the feature in the global feature set. */
clear_cpu_cap(&boot_cpu_data, X86_FEATURE_PGE);
}
put_online_cpus();
So, adjust_pge() clears X86_CR4_PGE, and boot cpu has X86_FEATURE_PGE
unset. This means, with static_cpu_has(X86_FEATURE_PGE) still 1, we
run into using cr4 for TLB flushing with no X86_CR4_PGE bit set (which
doesn't trigger the flush), whereas we should be using cr3 for flushing
from that point onwards instead.
I tried with this one, and things seem to work again:
diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index 6fa8594..fc5abff 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -188,7 +188,7 @@ static inline void __native_flush_tlb_single(unsigned long addr)
static inline void __flush_tlb_all(void)
{
- if (static_cpu_has(X86_FEATURE_PGE))
+ if (boot_cpu_has(X86_FEATURE_PGE))
__flush_tlb_global();
else
__flush_tlb();
Presumably coming from c109bf95992b ("x86/cpufeature: Remove cpu_has_pge")?
Thanks,
Daniel
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@suse.de> |
|---|---|
| Date | 2017-03-09 23:10 +0100 |
| Message-ID | <tjepY-2Qt-3@gated-at.bofh.it> |
| In reply to | #1596401 |
On Thu, Mar 09, 2017 at 10:55:47PM +0100, Borislav Petkov wrote:
> Can you make that:
>
> setup_clear_cpu_cap(X86_FEATURE_PGE);
>
> and see if it fixes your issue?
Hmm, in reading the thread a bit more, that might not work. If I see it
correctly, lguest does
clear_cpu_cap(&boot_cpu_data, X86_FEATURE_PGE);
after the alternatives have run and static_cpu_has() sites have already
been patched so clearing that bit won't bring anything.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
[toc] | [prev] | [next] | [standalone]
| From | Daniel Borkmann <daniel@iogearbox.net> |
|---|---|
| Date | 2017-03-09 23:20 +0100 |
| Message-ID | <tjezD-2W7-3@gated-at.bofh.it> |
| In reply to | #1596407 |
On 03/09/2017 11:07 PM, Borislav Petkov wrote: > On Thu, Mar 09, 2017 at 10:55:47PM +0100, Borislav Petkov wrote: >> Can you make that: >> >> setup_clear_cpu_cap(X86_FEATURE_PGE); >> >> and see if it fixes your issue? > > Hmm, in reading the thread a bit more, that might not work. If I see it > correctly, lguest does > > clear_cpu_cap(&boot_cpu_data, X86_FEATURE_PGE); > > after the alternatives have run and static_cpu_has() sites have already > been patched so clearing that bit won't bring anything. Yeah, I just tried that out and it had no effect unfortunately, the static_cpu_has() was still 1.
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@suse.de> |
|---|---|
| Date | 2017-03-09 23:50 +0100 |
| Message-ID | <tjf2G-363-19@gated-at.bofh.it> |
| In reply to | #1596412 |
On Thu, Mar 09, 2017 at 11:11:17PM +0100, Daniel Borkmann wrote:
> Yeah, I just tried that out and it had no effect unfortunately, the
> static_cpu_has() was still 1.
Right, just as I thought.
I guess we could return to doing boot_cpu_has() in __flush_tlb_all()
then. I mean, the timing-sensitivity argument is meh - killing global
TLB entries a bit faster doesn't bring me a whole lot when I have to go
and walk pagetable and reestablish them, which is the real price to pay
anyway.
Thanks.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
[toc] | [prev] | [next] | [standalone]
Page 1 of 2 [1] 2 Next page →
Back to top | Article view | linux.kernel
csiph-web