[PATCH] lib: Add test module for CONFIG_DEBUG_VIRTUAL

[PATCH] lib: Add test module for CONFIG_DEBUG_VIRTUAL

[Florian Fainelli]

Add a test module that allows testing that CONFIG_DEBUG_VIRTUAL works
correctly, at least that it can catch invalid calls to virt_to_phys()
against the non-linear kernel virtual address map.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 lib/Kconfig.debug        | 11 +++++++++++
 lib/Makefile             |  1 +
 lib/test_debug_virtual.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+)
 create mode 100644 lib/test_debug_virtual.c

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 98fe715522e8..d16bbc12429f 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1902,6 +1902,17 @@ config TEST_KMOD
 
 	  If unsure, say N.
 
+config TEST_DEBUG_VIRTUAL
+	tristate "Test CONFIG_DEBUG_VIRTUAL feature"
+	depends on DEBUG_VIRTUAL
+	help
+	  Test the kernel's ability to detect incorrect calls to
+	  virt_to_phys() done against the non-linear part of the
+	  kernel's virtual address map.
+
+	  If unsure, say N.
+
+
 source "samples/Kconfig"
 
 source "lib/Kconfig.kgdb"
diff --git a/lib/Makefile b/lib/Makefile
index 40c18372b301..469ce5e24e4f 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -62,6 +62,7 @@ obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
 obj-$(CONFIG_TEST_UUID) += test_uuid.o
 obj-$(CONFIG_TEST_PARMAN) += test_parman.o
 obj-$(CONFIG_TEST_KMOD) += test_kmod.o
+obj-$(CONFIG_TEST_DEBUG_VIRTUAL) += test_debug_virtual.o
 
 ifeq ($(CONFIG_DEBUG_KOBJECT),y)
 CFLAGS_kobject.o += -DDEBUG
diff --git a/lib/test_debug_virtual.c b/lib/test_debug_virtual.c
new file mode 100644
index 000000000000..b9cdeecc19dc
--- /dev/null
+++ b/lib/test_debug_virtual.c
@@ -0,0 +1,49 @@
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/export.h>
+#include <linux/mm.h>
+#include <linux/vmalloc.h>
+#include <linux/slab.h>
+#include <linux/sizes.h>
+
+#include <asm/page.h>
+#ifdef CONFIG_MIPS
+#include <asm/bootinfo.h>
+#endif
+
+struct foo {
+	unsigned int bar;
+};
+
+struct foo *foo;
+
+static int __init test_debug_virtual_init(void)
+{
+	phys_addr_t pa;
+	void *va;
+
+	va = (void *)VMALLOC_START;
+	pa = virt_to_phys(va);
+
+	pr_info("PA: %pa for VA: 0x%lx\n", &pa, (unsigned long)va);
+
+	foo = kzalloc(sizeof(*foo), GFP_KERNEL);
+	if (!foo)
+		return -ENOMEM;
+
+	pa = virt_to_phys(foo);
+	va = foo;
+	pr_info("PA: %pa for VA: 0x%lx\n", &pa, (unsigned long)va);
+
+	return 0;
+}
+module_init(test_debug_virtual_init);
+
+static void __exit test_debug_virtual_exit(void)
+{
+	kfree(foo);
+}
+module_exit(test_debug_virtual_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Test module for CONFIG_DEBUG_VIRTUAL");
-- 
2.9.3

Re: [PATCH] lib: Add test module for CONFIG_DEBUG_VIRTUAL

[Luis R. Rodriguez]

On Tue, Aug 08, 2017 at 09:40:26AM -0700, Florian Fainelli wrote:
> Add a test module that allows testing that CONFIG_DEBUG_VIRTUAL works
> correctly, at least that it can catch invalid calls to virt_to_phys()
> against the non-linear kernel virtual address map.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  lib/Kconfig.debug        | 11 +++++++++++
>  lib/Makefile             |  1 +
>  lib/test_debug_virtual.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 61 insertions(+)
>  create mode 100644 lib/test_debug_virtual.c
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 98fe715522e8..d16bbc12429f 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1902,6 +1902,17 @@ config TEST_KMOD
>  
>  	  If unsure, say N.
>  
> +config TEST_DEBUG_VIRTUAL
> +	tristate "Test CONFIG_DEBUG_VIRTUAL feature"
> +	depends on DEBUG_VIRTUAL
> +	help
> +	  Test the kernel's ability to detect incorrect calls to
> +	  virt_to_phys() done against the non-linear part of the
> +	  kernel's virtual address map.
> +
> +	  If unsure, say N.
> +
> +
>  source "samples/Kconfig"
>  
>  source "lib/Kconfig.kgdb"
> diff --git a/lib/Makefile b/lib/Makefile
> index 40c18372b301..469ce5e24e4f 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -62,6 +62,7 @@ obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
>  obj-$(CONFIG_TEST_UUID) += test_uuid.o
>  obj-$(CONFIG_TEST_PARMAN) += test_parman.o
>  obj-$(CONFIG_TEST_KMOD) += test_kmod.o
> +obj-$(CONFIG_TEST_DEBUG_VIRTUAL) += test_debug_virtual.o
>  
>  ifeq ($(CONFIG_DEBUG_KOBJECT),y)
>  CFLAGS_kobject.o += -DDEBUG
> diff --git a/lib/test_debug_virtual.c b/lib/test_debug_virtual.c
> new file mode 100644
> index 000000000000..b9cdeecc19dc
> --- /dev/null
> +++ b/lib/test_debug_virtual.c
> @@ -0,0 +1,49 @@
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/export.h>
> +#include <linux/mm.h>
> +#include <linux/vmalloc.h>
> +#include <linux/slab.h>
> +#include <linux/sizes.h>
> +
> +#include <asm/page.h>
> +#ifdef CONFIG_MIPS
> +#include <asm/bootinfo.h>
> +#endif
> +
> +struct foo {
> +	unsigned int bar;
> +};
> +
> +struct foo *foo;
> +
> +static int __init test_debug_virtual_init(void)
> +{
> +	phys_addr_t pa;
> +	void *va;
> +
> +	va = (void *)VMALLOC_START;
> +	pa = virt_to_phys(va);
> +
> +	pr_info("PA: %pa for VA: 0x%lx\n", &pa, (unsigned long)va);
> +
> +	foo = kzalloc(sizeof(*foo), GFP_KERNEL);
> +	if (!foo)
> +		return -ENOMEM;
> +
> +	pa = virt_to_phys(foo);
> +	va = foo;
> +	pr_info("PA: %pa for VA: 0x%lx\n", &pa, (unsigned long)va);

Should there be a tests here of some sort? When should this fail, why?
There is no docs on this self test, could one be added?

  Luis

> +
> +	return 0;
> +}
> +module_init(test_debug_virtual_init);
> +
> +static void __exit test_debug_virtual_exit(void)
> +{
> +	kfree(foo);
> +}
> +module_exit(test_debug_virtual_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Test module for CONFIG_DEBUG_VIRTUAL");
> -- 
> 2.9.3
> 
> 

-- 
Luis Rodriguez, SUSE LINUX GmbH
Maxfeldstrasse 5; D-90409 Nuernberg

Re: [PATCH] lib: Add test module for CONFIG_DEBUG_VIRTUAL

[Florian Fainelli]

On 08/08/2017 10:57 AM, Luis R. Rodriguez wrote:
> On Tue, Aug 08, 2017 at 09:40:26AM -0700, Florian Fainelli wrote:
>> Add a test module that allows testing that CONFIG_DEBUG_VIRTUAL works
>> correctly, at least that it can catch invalid calls to virt_to_phys()
>> against the non-linear kernel virtual address map.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---

>> +static int __init test_debug_virtual_init(void)
>> +{
>> +	phys_addr_t pa;
>> +	void *va;
>> +
>> +	va = (void *)VMALLOC_START;
>> +	pa = virt_to_phys(va);
>> +
>> +	pr_info("PA: %pa for VA: 0x%lx\n", &pa, (unsigned long)va);
>> +
>> +	foo = kzalloc(sizeof(*foo), GFP_KERNEL);
>> +	if (!foo)
>> +		return -ENOMEM;
>> +
>> +	pa = virt_to_phys(foo);
>> +	va = foo;
>> +	pr_info("PA: %pa for VA: 0x%lx\n", &pa, (unsigned long)va);
> 
> Should there be a tests here of some sort? When should this fail, why?

There is no test per-se, the kernel will produce warning with
CONFIG_DEBUG_VIRTUAL telling you that what you are doing is wrong.

> There is no docs on this self test, could one be added?

I suppose I could add one even though that just means pointing out the
code that produces the warning?
-- 
Florian

Re: [PATCH] lib: Add test module for CONFIG_DEBUG_VIRTUAL

[Luis R. Rodriguez]

On Tue, Aug 08, 2017 at 11:04:11AM -0700, Florian Fainelli wrote:
> On 08/08/2017 10:57 AM, Luis R. Rodriguez wrote:
> > On Tue, Aug 08, 2017 at 09:40:26AM -0700, Florian Fainelli wrote:
> >> Add a test module that allows testing that CONFIG_DEBUG_VIRTUAL works
> >> correctly, at least that it can catch invalid calls to virt_to_phys()
> >> against the non-linear kernel virtual address map.
> >>
> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> >> ---
> 
> >> +static int __init test_debug_virtual_init(void)
> >> +{
> >> +	phys_addr_t pa;
> >> +	void *va;
> >> +
> >> +	va = (void *)VMALLOC_START;
> >> +	pa = virt_to_phys(va);
> >> +
> >> +	pr_info("PA: %pa for VA: 0x%lx\n", &pa, (unsigned long)va);
> >> +
> >> +	foo = kzalloc(sizeof(*foo), GFP_KERNEL);
> >> +	if (!foo)
> >> +		return -ENOMEM;
> >> +
> >> +	pa = virt_to_phys(foo);
> >> +	va = foo;
> >> +	pr_info("PA: %pa for VA: 0x%lx\n", &pa, (unsigned long)va);
> > 
> > Should there be a tests here of some sort? When should this fail, why?
> 
> There is no test per-se, the kernel will produce warning with
> CONFIG_DEBUG_VIRTUAL telling you that what you are doing is wrong.
> 
> > There is no docs on this self test, could one be added?
> 
> I suppose I could add one even though that just means pointing out the
> code that produces the warning?

A /* note */ indicating what you just said above would suffice then but
typically tests return back to userspace an error, so another option
would be to see if one could get a return value that an error happened
and return that back to the module init. Grepping just for warning for
an error seems error prone.

  Luis

Re: [PATCH] lib: Add test module for CONFIG_DEBUG_VIRTUAL

[Kees Cook]

On Tue, Aug 8, 2017 at 11:35 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> On Tue, Aug 08, 2017 at 11:04:11AM -0700, Florian Fainelli wrote:
>> On 08/08/2017 10:57 AM, Luis R. Rodriguez wrote:
>> > On Tue, Aug 08, 2017 at 09:40:26AM -0700, Florian Fainelli wrote:
>> >> Add a test module that allows testing that CONFIG_DEBUG_VIRTUAL works
>> >> correctly, at least that it can catch invalid calls to virt_to_phys()
>> >> against the non-linear kernel virtual address map.
>> >>
>> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> >> ---
>>
>> >> +static int __init test_debug_virtual_init(void)
>> >> +{
>> >> +  phys_addr_t pa;
>> >> +  void *va;
>> >> +
>> >> +  va = (void *)VMALLOC_START;
>> >> +  pa = virt_to_phys(va);
>> >> +
>> >> +  pr_info("PA: %pa for VA: 0x%lx\n", &pa, (unsigned long)va);
>> >> +
>> >> +  foo = kzalloc(sizeof(*foo), GFP_KERNEL);
>> >> +  if (!foo)
>> >> +          return -ENOMEM;
>> >> +
>> >> +  pa = virt_to_phys(foo);
>> >> +  va = foo;
>> >> +  pr_info("PA: %pa for VA: 0x%lx\n", &pa, (unsigned long)va);
>> >
>> > Should there be a tests here of some sort? When should this fail, why?
>>
>> There is no test per-se, the kernel will produce warning with
>> CONFIG_DEBUG_VIRTUAL telling you that what you are doing is wrong.
>>
>> > There is no docs on this self test, could one be added?
>>
>> I suppose I could add one even though that just means pointing out the
>> code that produces the warning?
>
> A /* note */ indicating what you just said above would suffice then but
> typically tests return back to userspace an error, so another option
> would be to see if one could get a return value that an error happened
> and return that back to the module init. Grepping just for warning for
> an error seems error prone.

If the test depends on the kernel's response (i.e. WARN, BUG, panic)
that cannot be detected in the test itself, it may be better suited
for lkdtm (drivers/misc/lkdtm*) which is almost entirely comprised of
tests like that.

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH] lib: Add test module for CONFIG_DEBUG_VIRTUAL

[Florian Fainelli]

On 08/08/2017 12:58 PM, Kees Cook wrote:
> On Tue, Aug 8, 2017 at 11:35 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
>> On Tue, Aug 08, 2017 at 11:04:11AM -0700, Florian Fainelli wrote:
>>> On 08/08/2017 10:57 AM, Luis R. Rodriguez wrote:
>>>> On Tue, Aug 08, 2017 at 09:40:26AM -0700, Florian Fainelli wrote:
>>>>> Add a test module that allows testing that CONFIG_DEBUG_VIRTUAL works
>>>>> correctly, at least that it can catch invalid calls to virt_to_phys()
>>>>> against the non-linear kernel virtual address map.
>>>>>
>>>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>>>>> ---
>>>
>>>>> +static int __init test_debug_virtual_init(void)
>>>>> +{
>>>>> +  phys_addr_t pa;
>>>>> +  void *va;
>>>>> +
>>>>> +  va = (void *)VMALLOC_START;
>>>>> +  pa = virt_to_phys(va);
>>>>> +
>>>>> +  pr_info("PA: %pa for VA: 0x%lx\n", &pa, (unsigned long)va);
>>>>> +
>>>>> +  foo = kzalloc(sizeof(*foo), GFP_KERNEL);
>>>>> +  if (!foo)
>>>>> +          return -ENOMEM;
>>>>> +
>>>>> +  pa = virt_to_phys(foo);
>>>>> +  va = foo;
>>>>> +  pr_info("PA: %pa for VA: 0x%lx\n", &pa, (unsigned long)va);
>>>>
>>>> Should there be a tests here of some sort? When should this fail, why?
>>>
>>> There is no test per-se, the kernel will produce warning with
>>> CONFIG_DEBUG_VIRTUAL telling you that what you are doing is wrong.
>>>
>>>> There is no docs on this self test, could one be added?
>>>
>>> I suppose I could add one even though that just means pointing out the
>>> code that produces the warning?
>>
>> A /* note */ indicating what you just said above would suffice then but
>> typically tests return back to userspace an error, so another option
>> would be to see if one could get a return value that an error happened
>> and return that back to the module init. Grepping just for warning for
>> an error seems error prone.
> 
> If the test depends on the kernel's response (i.e. WARN, BUG, panic)
> that cannot be detected in the test itself, it may be better suited
> for lkdtm (drivers/misc/lkdtm*) which is almost entirely comprised of
> tests like that.

OK, do you have any specific coding styles, naming guidelines or
anything else that you would like to see for these lkdtm* modules?

On second thought it might be possible to produce both the warning and
check the virtual address against PAGE_OFFSET and if it is outside,
return an error during module_init(), I will experiment with that a
little bit.
-- 
Florian