[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170119154658.GD27312@n2100.armlinux.org.uk>
Date: Thu, 19 Jan 2017 15:46:58 +0000
From: Russell King - ARM Linux <linux@...linux.org.uk>
To: Jinbum Park <jinb.park7@...il.com>
Cc: tglx@...utronix.de, mingo@...hat.com, hpa@...or.com, x86@...nel.org,
keescook@...omium.org, arjan@...ux.intel.com,
akpm@...uxfoundation.org, linux-kernel@...r.kernel.org,
linux-mm@...ck.org, labbott@...hat.com,
kernel-hardening@...ts.openwall.com, mark.rutland@....com,
kernel-janitors@...r.kernel.org
Subject: Re: [PATCH v2] mm: add arch-independent testcases for RODATA
On Fri, Jan 20, 2017 at 12:39:20AM +0900, Jinbum Park wrote:
> diff --git a/mm/Kconfig.debug b/mm/Kconfig.debug
> index afcc550..e4f22ce 100644
> --- a/mm/Kconfig.debug
> +++ b/mm/Kconfig.debug
> @@ -90,3 +90,9 @@ config DEBUG_PAGE_REF
> careful when enabling this feature because it adds about 30 KB to the
> kernel code. However the runtime performance overhead is virtually
> nil until the tracepoints are actually enabled.
> +
> +config DEBUG_RODATA_TEST
> + bool "Testcase for the marking rodata read-only"
> + depends on DEBUG_RODATA
> + ---help---
> + This option enables a testcase for the setting rodata read-only.
> \ No newline at end of file
It's worth reviewing your own patches before sending them out for
things like this (please ensure that all files are not left without
a newline at the end.)
> diff --git a/mm/rodata_test.c b/mm/rodata_test.c
> new file mode 100644
> index 0000000..fb953c0
> --- /dev/null
> +++ b/mm/rodata_test.c
> @@ -0,0 +1,64 @@
> +/*
> + * rodata_test.c: functional test for mark_rodata_ro function
> + *
> + * (C) Copyright 2008 Intel Corporation
> + * Author: Arjan van de Ven <arjan@...ux.intel.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; version 2
> + * of the License.
> + */
> +#include <asm/uaccess.h>
> +#include <asm/sections.h>
> +
> +const int rodata_test_data = 0xC3;
> +EXPORT_SYMBOL_GPL(rodata_test_data);
> +
> +void rodata_test(void)
> +{
> + unsigned long start, end, rodata_addr;
> + int zero = 0;
> +
> + /* prepare test */
> + rodata_addr = ((unsigned long)&rodata_test_data);
> +
> + /* test 1: read the value */
> + /* If this test fails, some previous testrun has clobbered the state */
> + if (!rodata_test_data) {
> + pr_err("rodata_test: test 1 fails (start data)\n");
> + return;
> + }
> +
> + /* test 2: write to the variable; this should fault */
> + /*
> + * This must be written in assembly to be able to catch the
> + * exception that is supposed to happen in the correct case.
> + *
> + * So that put_user macro is used to write arch-independent assembly.
> + */
> + if (!put_user(zero, (int *)rodata_addr)) {
> + pr_err("rodata_test: test data was not read only\n");
> + return;
> + }
I don't think this is going to do what you think - at least not on sane
architectures. put_user() to kernel space is denied, even if the
location is writable to normal accesses within the kernel.
put_user() and get_user() are for accessing user supplied pointers,
which means it has built-in security to prevent userspace passing in
kernel-space pointers and using that as a way to read or modify kernel
space.
I think you want to use probe_kernel_write() here.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
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.
