@@ -230,7 +230,7 @@ static inline int hugepd_ok(hugepd_t hpd)return((hpd_val(hpd)&0x4)!=0);#else/* We clear the top bit to indicate hugepd */-return((hpd_val(hpd)&PD_HUGE)==0);+return(hpd_val(hpd)&&(hpd_val(hpd)&PD_HUGE)==0);#endif}
From: Scott Wood <oss@buserror.net> Date: 2017-02-17 10:08:26
On Thu, 2017-02-16 at 09:11 -0600, laurentiu.tudor@nxp.com wrote:
From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
On 32-bit book-e machines, hugepd_ok() does not take
into account null hugepd values, causing this crash at boot:
@@ -230,7 +230,7 @@ static inline int hugepd_ok(hugepd_t hpd)
return ((hpd_val(hpd) & 0x4) != 0);
#else
/* We clear the top bit to indicate hugepd */
- return ((hpd_val(hpd) & PD_HUGE) == 0);
+ return (hpd_val(hpd) && (hpd_val(hpd) & PD_HUGE) == 0);
#endif
}
Any reason why this can't go back to being "hpd_val(hpd) > 0"? Why was nohash
changed to begin with? I don't expect nohash (or at least fsl-book3e) will
ever have a pagetable that is not native-endian, and "> 0" is consistent with
what the TLB miss code is doing.
Also, the patch that broke this was tagged for stable (which again raises the
question of why an extraneous change was made) so this patch needs to be as
well.
-Scott
@@ -230,7 +230,7 @@ static inline int hugepd_ok(hugepd_t hpd)return((hpd_val(hpd)&0x4)!=0);#else/* We clear the top bit to indicate hugepd */-return((hpd_val(hpd)&PD_HUGE)==0);+return(hpd_val(hpd)&&(hpd_val(hpd)&PD_HUGE)==0);#endif}
From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
On 32-bit book-e machines, hugepd_ok() does not take
into account null hugepd values, causing this crash at boot:
Unable to handle kernel paging request for data at address 0x80000000
Faulting instruction address: 0xc00182a8
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=3D24
CoreNet Generic
Modules linked in:
CPU: 1 PID: 1 Comm: swapper/0 Tainted: G W 4.10.0-rc8-00016=
-g69b1f87 #11
quoted
task: e5050000 task.stack: e5058000
NIP: c00182a8 LR: c001829c CTR: 00007ffe
REGS: e5059c50 TRAP: 0300 Tainted: G W (4.10.0-rc8-00016=
Which code path is that. That null should be filtered by the if
(pmd_none(pmd)) check in find_linux_pte_or_hugepte right ?
I haven't characterized the issue in detail as i wanted to get the patch=20
out ASAP.
I only noticed that the previous check, that is:
"signed hpd_val > 0"
vs the new one, that is:
"unsigned hpd_val & PD_HUGE =3D=3D 0"
evaluate differently for a value of zero: old expression evaluates
to false and the new one to true.
---
Best Regards, Laurentiu=
Hi,
Some more information on the crash, inline.
On 02/17/2017 02:18 PM, Aneesh Kumar K.V wrote:
laurentiu.tudor@nxp.com writes:
quoted
From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
On 32-bit book-e machines, hugepd_ok() does not take
into account null hugepd values, causing this crash at boot:
Unable to handle kernel paging request for data at address 0x80000000
Faulting instruction address: 0xc00182a8
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=3D24
CoreNet Generic
Modules linked in:
CPU: 1 PID: 1 Comm: swapper/0 Tainted: G W 4.10.0-rc8-00016=
-g69b1f87 #11
quoted
task: e5050000 task.stack: e5058000
NIP: c00182a8 LR: c001829c CTR: 00007ffe
REGS: e5059c50 TRAP: 0300 Tainted: G W (4.10.0-rc8-00016=
Which code path is that. That null should be filtered by the if
(pmd_none(pmd)) check in find_linux_pte_or_hugepte right ?
The crash happens when __find_linux_pte_or_hugepte() calls hugepd_ok(),
on this line [1]. It's triggered when __find_linux_pte_or_hugepte() is
first called, when the kernel tries to spawn the init process. The input
effective address (ea arg) is bffffff1. This is the call stack:
[e5059cd0] [c0017b60] __find_linux_pte_or_hugepte+0x60/0x120 (unreliable)
[e5059d00] [c001832c] follow_huge_addr+0x2c/0xf0
[e5059d20] [c0107590] follow_page_mask+0x40/0x3c0
[e5059d80] [c01079d8] __get_user_pages+0xc8/0x420
[e5059de0] [c01081fc] get_user_pages_remote+0x8c/0x230
[e5059e30] [c013f210] copy_strings+0x110/0x3a0
[e5059ea0] [c013f4cc] copy_strings_kernel+0x2c/0x50
[e5059ec0] [c01413c4] do_execveat_common+0x474/0x620
[e5059f10] [c014159c] do_execve+0x2c/0x40
[e5059f20] [c0001f68] try_to_run_init_process+0x18/0x60
[e5059f30] [c000289c] kernel_init+0xcc/0x120
[e5059f40] [c000f1e8] ret_from_kernel_thread+0x5c/0x64
Thanks in advance for any pointers.
[1]=20
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/po=
werpc/mm/hugetlbpage.c#n918
---
Best Regards, Laurentiu=
Which code path is that. That null should be filtered by the if
(pmd_none(pmd)) check in find_linux_pte_or_hugepte right ?
The crash happens when __find_linux_pte_or_hugepte() calls hugepd_ok(),
on this line [1]. It's triggered when __find_linux_pte_or_hugepte() is
first called, when the kernel tries to spawn the init process. The input
effective address (ea arg) is bffffff1. This is the call stack:
What is the pmd value? There's a pmd_none() check before that line.
That said, regardless of what's going wrong here, it would be simpler and more
robust if is_hugepd() returned false for empty ptes rather than assuming the
caller explicitly checked pmd_none().
-Scott
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2017-03-01 11:09:25
Scott Wood [off-list ref] writes:
On Tue, 2017-02-28 at 14:55 +0000, Laurentiu Tudor wrote:
quoted
On 02/17/2017 02:18 PM, Aneesh Kumar K.V wrote:
quoted
laurentiu.tudor@nxp.com writes:
quoted
From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
=20
On 32-bit book-e machines, hugepd_ok() does not take
into account null hugepd values, causing this crash at boot:
=20
Unable to handle kernel paging request for data at address 0x80000000
Faulting instruction address: 0xc00182a8
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=3D24
CoreNet Generic
Modules linked in:
CPU: 1 PID: 1 Comm: swapper/0 Tainted: G=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=20
Which code path is that. That null should be filtered by the if
(pmd_none(pmd)) check in find_linux_pte_or_hugepte right ?
The crash happens when __find_linux_pte_or_hugepte() calls hugepd_ok(),
on this line [1]. It's triggered when __find_linux_pte_or_hugepte() is
first called, when the kernel tries to spawn the init process. The input
effective address (ea arg) is bffffff1. This is the call stack:
What is the pmd value? =C2=A0There's a pmd_none() check before that line.
It's a pgd, so a pgd_none() check.
But that does nothing because this is 32-bit, 4K PAGE_SIZE, which uses
pgtable-nopmd.h and pgtable-nopud.h, so pgd_none() is just:
int pgd_none(pgd_t pgd) { return 0; }
That said, regardless of what's going wrong here, it would be simpler and=
more
robust if is_hugepd() returned false for empty ptes rather than assuming =
the
caller explicitly checked pmd_none().
Yeah, in fact it has to, because of the above.
So Laurentiu's patch is pretty much the correct fix.
cheers