This is a short series of fixes and cleanups found when reading the
code, no functional changes.
Jiang Biao (5):
x86/pti: check the return value of pti_user_pagetable_walk_p4d
x86/pti: check the return value of pti_user_pagetable_walk_pmd
x86/pti: make pti_set_kernel_image_nonglobal static
x86/pti: warn for unknown pti boot options
x86/pti: constify address parameters
arch/x86/mm/pti.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
--
2.7.4
pti_set_kernel_image_nonglobal() is only used in pti.c, make it
static.
Signed-off-by: Jiang Biao <[email protected]>
---
arch/x86/mm/pti.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/mm/pti.c b/arch/x86/mm/pti.c
index bb6f608..a76b2cc 100644
--- a/arch/x86/mm/pti.c
+++ b/arch/x86/mm/pti.c
@@ -473,7 +473,7 @@ void pti_clone_kernel_text(void)
* the other set_memory.h functions. Just extern it.
*/
extern int set_memory_nonglobal(unsigned long addr, int numpages);
-void pti_set_kernel_image_nonglobal(void)
+static void pti_set_kernel_image_nonglobal(void)
{
/*
* The identity map is created with PMDs, regardless of the
--
2.7.4
pti_user_pagetable_walk_p4d() may return NULL, we should check the
return value to avoid NULL pointer dereference.
Signed-off-by: Jiang Biao <[email protected]>
---
arch/x86/mm/pti.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/x86/mm/pti.c b/arch/x86/mm/pti.c
index 4d418e7..be9e5bc 100644
--- a/arch/x86/mm/pti.c
+++ b/arch/x86/mm/pti.c
@@ -195,8 +195,10 @@ static p4d_t *pti_user_pagetable_walk_p4d(unsigned long address)
static pmd_t *pti_user_pagetable_walk_pmd(unsigned long address)
{
gfp_t gfp = (GFP_KERNEL | __GFP_NOTRACK | __GFP_ZERO);
- p4d_t *p4d = pti_user_pagetable_walk_p4d(address);
pud_t *pud;
+ p4d_t *p4d = pti_user_pagetable_walk_p4d(address);
+ if (WARN_ON(!p4d))
+ return NULL;
BUILD_BUG_ON(p4d_large(*p4d) != 0);
if (p4d_none(*p4d)) {
@@ -354,6 +356,9 @@ static void __init pti_clone_p4d(unsigned long addr)
pgd_t *kernel_pgd;
user_p4d = pti_user_pagetable_walk_p4d(addr);
+ if (WARN_ON(!user_p4d))
+ return;
+
kernel_pgd = pgd_offset_k(addr);
kernel_p4d = p4d_offset(kernel_pgd, addr);
*user_p4d = *kernel_p4d;
--
2.7.4
Addresses passed in pti_user_pagetable_walk_*() are not supposed to
change at runtime, make them const to aviod future slipups.
Signed-off-by: Jiang Biao <[email protected]>
---
arch/x86/mm/pti.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/mm/pti.c b/arch/x86/mm/pti.c
index a368656..ac2cbdd 100644
--- a/arch/x86/mm/pti.c
+++ b/arch/x86/mm/pti.c
@@ -164,7 +164,7 @@ pgd_t __pti_set_user_pgd(pgd_t *pgdp, pgd_t pgd)
*
* Returns a pointer to a P4D on success, or NULL on failure.
*/
-static p4d_t *pti_user_pagetable_walk_p4d(unsigned long address)
+static p4d_t *pti_user_pagetable_walk_p4d(const unsigned long address)
{
pgd_t *pgd = kernel_to_user_pgdp(pgd_offset_k(address));
gfp_t gfp = (GFP_KERNEL | __GFP_NOTRACK | __GFP_ZERO);
@@ -192,7 +192,7 @@ static p4d_t *pti_user_pagetable_walk_p4d(unsigned long address)
*
* Returns a pointer to a PMD on success, or NULL on failure.
*/
-static pmd_t *pti_user_pagetable_walk_pmd(unsigned long address)
+static pmd_t *pti_user_pagetable_walk_pmd(const unsigned long address)
{
gfp_t gfp = (GFP_KERNEL | __GFP_NOTRACK | __GFP_ZERO);
pud_t *pud;
@@ -236,7 +236,7 @@ static pmd_t *pti_user_pagetable_walk_pmd(unsigned long address)
*
* Returns a pointer to a PTE on success, or NULL on failure.
*/
-static __init pte_t *pti_user_pagetable_walk_pte(unsigned long address)
+static __init pte_t *pti_user_pagetable_walk_pte(const unsigned long address)
{
gfp_t gfp = (GFP_KERNEL | __GFP_NOTRACK | __GFP_ZERO);
pte_t *pte;
--
2.7.4
When using unknown pti boot options other than on/off/auto, we
select auto silently, which is sometimes confusing. Add warning for
unknown pti boot options like we do in
spectre_v2_select_mitigation().
Signed-off-by: Jiang Biao <[email protected]>
---
arch/x86/mm/pti.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/x86/mm/pti.c b/arch/x86/mm/pti.c
index a76b2cc..a368656 100644
--- a/arch/x86/mm/pti.c
+++ b/arch/x86/mm/pti.c
@@ -92,14 +92,14 @@ void __init pti_check_boottime_disable(void)
pti_mode = PTI_FORCE_OFF;
pti_print_if_insecure("disabled on command line.");
return;
- }
- if (ret == 2 && !strncmp(arg, "on", 2)) {
+ } else if (ret == 2 && !strncmp(arg, "on", 2)) {
pti_mode = PTI_FORCE_ON;
pti_print_if_secure("force enabled on command line.");
goto enable;
- }
- if (ret == 4 && !strncmp(arg, "auto", 4)) {
- pti_mode = PTI_AUTO;
+ } else if (ret == 4 && !strncmp(arg, "auto", 4)) {
+ goto autosel;
+ } else {
+ pr_err("unknown option (%s). Switching to AUTO select\n", arg);
goto autosel;
}
}
--
2.7.4
pti_user_pagetable_walk_pmd() may return NULL, we should check the
return value in pti_user_pagetable_walk_pte() to avoid NULL pointer
dereference like it is checked in pti_clone_pmds().
Signed-off-by: Jiang Biao <[email protected]>
---
arch/x86/mm/pti.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/x86/mm/pti.c b/arch/x86/mm/pti.c
index be9e5bc..bb6f608 100644
--- a/arch/x86/mm/pti.c
+++ b/arch/x86/mm/pti.c
@@ -239,8 +239,10 @@ static pmd_t *pti_user_pagetable_walk_pmd(unsigned long address)
static __init pte_t *pti_user_pagetable_walk_pte(unsigned long address)
{
gfp_t gfp = (GFP_KERNEL | __GFP_NOTRACK | __GFP_ZERO);
- pmd_t *pmd = pti_user_pagetable_walk_pmd(address);
pte_t *pte;
+ pmd_t *pmd = pti_user_pagetable_walk_pmd(address);
+ if (WARN_ON(!pmd))
+ return NULL;
/* We can't do anything sensible if we hit a large mapping. */
if (pmd_large(*pmd)) {
--
2.7.4
On 07/15/2018 09:03 PM, Jiang Biao wrote:
> diff --git a/arch/x86/mm/pti.c b/arch/x86/mm/pti.c
> index 4d418e7..be9e5bc 100644
> --- a/arch/x86/mm/pti.c
> +++ b/arch/x86/mm/pti.c
> @@ -195,8 +195,10 @@ static p4d_t *pti_user_pagetable_walk_p4d(unsigned long address)
> static pmd_t *pti_user_pagetable_walk_pmd(unsigned long address)
> {
> gfp_t gfp = (GFP_KERNEL | __GFP_NOTRACK | __GFP_ZERO);
> - p4d_t *p4d = pti_user_pagetable_walk_p4d(address);
> pud_t *pud;
> + p4d_t *p4d = pti_user_pagetable_walk_p4d(address);
> + if (WARN_ON(!p4d))
> + return NULL;
First of all, I don't think we need the (new) warning here.
pti_user_pagetable_walk_p4d() only returns NULL if you try it on a
userspace _address_ or the page allocation fails. It already warns on
the address case.
If you think the allocation path needs a warning, please do it as close
as possible to the _source_ of the warning (the failed allocation), not
in the caller.
This goes for all the variations on this that you have in the set.
On 07/15/2018 09:03 PM, Jiang Biao wrote:
> When using unknown pti boot options other than on/off/auto, we
> select auto silently, which is sometimes confusing. Add warning for
> unknown pti boot options like we do in
> spectre_v2_select_mitigation().
There's not a lot of precedent for this one. We have gazillions of boot
options and I don't know of many places that we do this. While this
isn't a horrible idea, I don't look forward to the patches will do this
for every other option in the kernel.
On 07/15/2018 09:03 PM, Jiang Biao wrote:
> -static p4d_t *pti_user_pagetable_walk_p4d(unsigned long address)
> +static p4d_t *pti_user_pagetable_walk_p4d(const unsigned long address)
> {
> pgd_t *pgd = kernel_to_user_pgdp(pgd_offset_k(address));
> gfp_t gfp = (GFP_KERNEL | __GFP_NOTRACK | __GFP_ZERO);
I'm also a bit ambivalent on this one. While this is _correct_ and not
something I think we actively discourage, it's not something that we're
horribly diligent about doing universally in the kernel.
On 07/15/2018 09:03 PM, Jiang Biao wrote:
> pti_set_kernel_image_nonglobal() is only used in pti.c, make it
> static.
I went back and forth on where this gets called from so it makes sense
that this would get screwed up. grep confirms it is not getting called
from elsewhere.
Acked-by: Dave Hansen <[email protected]>
Commit-ID: 21279157efffe5e7258483809942d576cb802768
Gitweb: https://git.kernel.org/tip/21279157efffe5e7258483809942d576cb802768
Author: Jiang Biao <[email protected]>
AuthorDate: Mon, 16 Jul 2018 12:03:38 +0800
Committer: Thomas Gleixner <[email protected]>
CommitDate: Mon, 16 Jul 2018 17:59:57 +0200
x86/pti: Make pti_set_kernel_image_nonglobal() static
pti_set_kernel_image_nonglobal() is only used in pti.c, make it static.
Signed-off-by: Jiang Biao <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Acked-by: Dave Hansen <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Link: https://lkml.kernel.org/r/[email protected]
---
arch/x86/mm/pti.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/mm/pti.c b/arch/x86/mm/pti.c
index 4d418e705878..9277e9ba92b5 100644
--- a/arch/x86/mm/pti.c
+++ b/arch/x86/mm/pti.c
@@ -466,7 +466,7 @@ void pti_clone_kernel_text(void)
* the other set_memory.h functions. Just extern it.
*/
extern int set_memory_nonglobal(unsigned long addr, int numpages);
-void pti_set_kernel_image_nonglobal(void)
+static void pti_set_kernel_image_nonglobal(void)
{
/*
* The identity map is created with PMDs, regardless of the