[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0c858bb2-9be8-ff77-d5ec-7f74700cdb85@c-s.fr>
Date: Wed, 26 Feb 2020 08:29:11 +0100
From: Christophe Leroy <christophe.leroy@....fr>
To: Russell Currey <ruscur@...sell.cc>, linuxppc-dev@...ts.ozlabs.org
Cc: jniethe5@...il.com, joel@....id.au, mpe@...erman.id.au,
ajd@...ux.ibm.com, dja@...ens.net, npiggin@...il.com,
kernel-hardening@...ts.openwall.com
Subject: Re: [PATCH v5 8/8] powerpc/mm: Disable set_memory() routines when
strict RWX isn't enabled
Le 26/02/2020 à 07:35, Russell Currey a écrit :
> There are a couple of reasons that the set_memory() functions are
> problematic when STRICT_KERNEL_RWX isn't enabled:
>
> - The linear mapping is a different size and apply_to_page_range()
> may modify a giant section, breaking everything
> - patch_instruction() doesn't know to work around a page being marked
> RO, and will subsequently crash
>
> The latter can be replicated by building a kernel with the set_memory()
> patches but with STRICT_KERNEL_RWX off and running ftracetest.
I agree with Andrew, those changes should go into patch 1.
>
> Reported-by: Jordan Niethe <jniethe5@...il.com>
> Signed-off-by: Russell Currey <ruscur@...sell.cc>
> ---
> v5: Apply to both set_memory_attr() and change_memory_attr()
See my comments of v4, additional comments below
> v4: New
>
> arch/powerpc/mm/pageattr.c | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
> index ee6b5e3b7604..49b8e2e0581d 100644
> --- a/arch/powerpc/mm/pageattr.c
> +++ b/arch/powerpc/mm/pageattr.c
> @@ -64,13 +64,18 @@ static int change_page_attr(pte_t *ptep, unsigned long addr, void *data)
>
> int change_memory_attr(unsigned long addr, int numpages, long action)
> {
> - unsigned long start = ALIGN_DOWN(addr, PAGE_SIZE);
> - unsigned long sz = numpages * PAGE_SIZE;
> + unsigned long start, size;
> +
> + if (!IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
> + return 0;
>
> if (!numpages)
> return 0;
>
> - return apply_to_page_range(&init_mm, start, sz, change_page_attr, (void *)action);
> + start = ALIGN_DOWN(addr, PAGE_SIZE);
> + size = numpages * PAGE_SIZE;
> +
> + return apply_to_page_range(&init_mm, start, size, change_page_attr, (void *)action);
You don't need to move start and sz initialisation, neither you need to
change the name of sz to size.
If you want to rename sz to size, do it in the initial patch, but take
care of the length of the lines. IIRC I used a short name to have the
line fit on a single line with no more than 90 chars.
Christophe
> }
>
> /*
> @@ -96,12 +101,17 @@ static int set_page_attr(pte_t *ptep, unsigned long addr, void *data)
>
> int set_memory_attr(unsigned long addr, int numpages, pgprot_t prot)
> {
> - unsigned long start = ALIGN_DOWN(addr, PAGE_SIZE);
> - unsigned long sz = numpages * PAGE_SIZE;
> + unsigned long start, size;
> +
> + if (!IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
> + return 0;
>
> if (!numpages)
> return 0;
>
> - return apply_to_page_range(&init_mm, start, sz, set_page_attr,
> + start = ALIGN_DOWN(addr, PAGE_SIZE);
> + size = numpages * PAGE_SIZE;
> +
> + return apply_to_page_range(&init_mm, start, size, set_page_attr,
> (void *)pgprot_val(prot));
> }
>
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
