lkdtm: add tests for atomic over-/underflow

Thank you Kees! I applied the commit to our hardened_atomic_on_next branch and it will be included into the next rfc.

Quoted 1 line · Elena

Thank you Kees! I applied the commit to our hardened_atomic_on_next branch and it will be included into the next rfc.

Cool, thanks. I assume this should get atomic64_t and local_t tests as well?

Quoted 6 lines · Kees Cook

Quoted 1 line · Elena

Thank you Kees! I applied the commit to our hardened_atomic_on_next branch and it will be included into the next rfc.

Cool, thanks. I assume this should get atomic64_t and local_t tests as well?

Yes, I'm currently compiling a build with atomic64_t and local_t tests added. With the improved lkdtm macros its much easier to add the extra types, thank you Kees!

Quoted 8 lines · Hans Liljestrand

Quoted 6 lines · Kees Cook

Quoted 1 line · Elena

Thank you Kees! I applied the commit to our hardened_atomic_on_next branch and it will be included into the next rfc.

Cool, thanks. I assume this should get atomic64_t and local_t tests as well?

Quoted 3 lines

Yes, I'm currently compiling a build with atomic64_t and local_t tests added. With the improved lkdtm macros its much easier to add the extra types, thank you Kees!

Sure thing! I may have yet-another patch for this, as I didn't like repeating the same things in three files whenever a new test was added. Moar macro magick!

-Kees

Signature · 2 lines

Kees Cook
Nexus Security

From c516b50b4764c5c1ba0dd39e3a5022d026e35514 Mon Sep 17 00:00:00 2001

Quoted 13 lines · Kees Cook

Quoted 11 lines · Hans Liljestrand

Quoted 6 lines · Kees Cook

Quoted 1 line · Elena

Thank you Kees! I applied the commit to our hardened_atomic_on_next branch and it will be included into the next rfc.

Cool, thanks. I assume this should get atomic64_t and local_t tests as well?

Yes, I'm currently compiling a build with atomic64_t and local_t tests added. With the improved lkdtm macros its much easier to add the extra types, thank you Kees!

Quoted 3 lines

Sure thing! I may have yet-another patch for this, as I didn't like repeating the same things in three files whenever a new test was added. Moar macro magick!

It would be nice to expose atomic* variables to userspace via debugfs so that we can confirm that the values will not be changed if overflowed. See the attached patch.

We will be able to check the test result:

 # /bin/echo ATOMIC_ADD_OVERFLOW > /debug/provoke-crash/DIRECT
 # echo $?
 # if [ cat /debug/provoke-crash/atomic -eq INT_MAX ]; then
 # echo PASS ; fi

Thanks,
-Takahiro AKASHI

Quoted 6 lines

-Kees

Signature · 2 lines

Kees Cook
Nexus Security

===8<===
From c516b50b4764c5c1ba0dd39e3a5022d026e35514 Mon Sep 17 00:00:00 2001
From: AKASHI Takahiro <[email protected]>
Date: Fri, 28 Oct 2016 10:55:50 +0900
Subject: [PATCH] lkdtm: expose atomic variables via debugfs

Signed-off-by AKASHI Takahiro <[email protected]>

---
 drivers/misc/lkdtm.h      |  2 ++
 drivers/misc/lkdtm_bugs.c | 75 +++++++++++++++++++++++++++++++++++++----------
 drivers/misc/lkdtm_core.c |  3 ++
 3 files changed, 64 insertions(+), 16 deletions(-)

drivers/misc/lkdtm.h +2 -0

index 0ef66ff..b4dd231 100644
@@ -3,10 +3,12 @@
 
 #define pr_fmt(fmt) "lkdtm: " fmt
 
+#include <linux/fs.h>
 #include <linux/kernel.h>
 
 /* lkdtm_bugs.c */
 void __init lkdtm_bugs_init(int *recur_param);
+void __init lkdtm_bugs_init2(struct dentry *parent);
 void lkdtm_PANIC(void);
 void lkdtm_BUG(void);
 void lkdtm_WARNING(void);

drivers/misc/lkdtm_bugs.c +59 -16

index 7b4067b..dd5003f 100644
@@ -5,6 +5,8 @@
  * test source files.
  */
 #include "lkdtm.h"
+#include <linux/debugfs.h>
+#include <linux/fs.h>
 #include <linux/sched.h>
 
 /*

@@ -35,6 +37,33 @@ static int recursive_loop(int remaining)
 		return recursive_loop(remaining - 1);
 }
 
+/* from fs/debugfs/file.c */
+static int debugfs_atomic_long_t_set(void *data, u64 val)
+{
+	atomic_long_set((atomic_long_t *)data, val);
+	return 0;
+}
+
+static int debugfs_atomic_long_t_get(void *data, u64 *val)
+{
+	*val = atomic_long_read((atomic_long_t *)data);
+	return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(fops_atomic_long_t, debugfs_atomic_long_t_get,
+			debugfs_atomic_long_t_set, "%lld\n");
+
+static struct dentry *debugfs_create_atomic_long_t(const char *name,
+				umode_t mode,
+				struct dentry *parent, atomic_long_t *value)
+{
+	return debugfs_create_file_unsafe(name, mode, parent, value,
+					&fops_atomic_long_t);
+}
+
+static atomic_t atomic_var = ATOMIC_INIT(0);
+static atomic_long_t atomic_long_var = ATOMIC_LONG_INIT(0);
+
 /* If the depth is negative, use the default, otherwise keep parameter. */
 void __init lkdtm_bugs_init(int *recur_param)
 {

@@ -44,6 +73,20 @@ void __init lkdtm_bugs_init(int *recur_param)
 		recur_count = *recur_param;
 }
 
+void __init lkdtm_bugs_init2(struct dentry *parent)
+{
+	struct dentry *de;
+
+	de = debugfs_create_atomic_t("atomic", 0644, parent, &atomic_var);
+	if (de == NULL)
+		pr_err("could not create atomic dentry under debugfs\n");
+
+	de = debugfs_create_atomic_long_t("atomic-long", 0644, parent,
+							&atomic_long_var);
+	if (de == NULL)
+		pr_err("could not create atomic-long dentry under debugfs\n");
+}
+
 void lkdtm_PANIC(void)
 {
 	panic("dumptest");

@@ -125,53 +168,53 @@ void lkdtm_HUNG_TASK(void)
 
 #define ATOMIC_LKDTM_MIN(tag,fun) void lkdtm_ATOMIC_##tag(void)	\
 {									\
-	atomic_t atomic = ATOMIC_INIT(INT_MIN);				\
+	atomic_set(&atomic_var, INT_MIN);				\
 									\
 	pr_info("attempting good atomic_" #fun "\n");			\
-	atomic_inc(&atomic);						\
-	TEST_FUNC(&atomic);						\
+	atomic_inc(&atomic_var);					\
+	TEST_FUNC(&atomic_var);						\
 									\
 	pr_info("attempting bad atomic_" #fun "\n");			\
-	TEST_FUNC(&atomic);						\
+	TEST_FUNC(&atomic_var);						\
 }
 
 #define ATOMIC_LKDTM_MAX(tag,fun,...)					\
 void lkdtm_ATOMIC_##tag(void)						\
 {									\
-	atomic_t atomic = ATOMIC_INIT(INT_MAX);				\
+	atomic_set(&atomic_var, INT_MAX);				\
 									\
 	pr_info("attempting good atomic_" #fun "\n");			\
-	atomic_dec(&atomic);						\
-	TEST_FUNC(&atomic);						\
+	atomic_dec(&atomic_var);					\
+	TEST_FUNC(&atomic_var);						\
 									\
 	pr_info("attempting bad atomic_" #fun "\n");			\
-	TEST_FUNC(&atomic);						\
+	TEST_FUNC(&atomic_var);						\
 }
 
 #define ATOMIC_LKDTM_LONG_MIN(tag,fun,...)				\
 void lkdtm_ATOMIC_LONG_##tag(void)					\
 {									\
-	atomic_long_t atomic  = ATOMIC_LONG_INIT(LONG_MIN);		\
+	atomic_long_set(&atomic_long_var, LONG_MIN);			\
 									\
 	pr_info("attempting good atomic_long_" #fun "\n");		\
-	atomic_long_inc(&atomic);					\
-	TEST_FUNC(&atomic);						\
+	atomic_long_inc(&atomic_long_var);				\
+	TEST_FUNC(&atomic_long_var);					\
 									\
 	pr_info("attempting bad atomic_long_" #fun "\n");		\
-	TEST_FUNC(&atomic);						\
+	TEST_FUNC(&atomic_long_var);					\
 }
 
 #define ATOMIC_LKDTM_LONG_MAX(tag,fun,...)				\
 void lkdtm_ATOMIC_LONG_##tag(void)					\
 {									\
-	atomic_long_t atomic = ATOMIC_LONG_INIT(LONG_MAX);		\
+	atomic_long_set(&atomic_long_var, LONG_MAX);			\
 									\
 	pr_info("attempting good atomic_long_" #fun "\n");		\
-	atomic_long_dec(&atomic);					\
-	TEST_FUNC(&atomic);						\
+	atomic_long_dec(&atomic_long_var);				\
+	TEST_FUNC(&atomic_long_var);					\
 									\
 	pr_info("attempting bad atomic_long_" #fun "\n");		\
-	TEST_FUNC(&atomic);						\
+	TEST_FUNC(&atomic_long_var);					\
 }
 
 #define TEST_FUNC(x) atomic_dec(x)

drivers/misc/lkdtm_core.c +3 -0

index 01d8540..6f5f9c2 100644
@@ -536,6 +536,9 @@ static int __init lkdtm_module_init(void)
 		pr_info("No crash points registered, enable through debugfs\n");
 	}
 
+	/* misc setup */
+	lkdtm_bugs_init2(lkdtm_debugfs_root);
+
 	return 0;
 
 out_err:

Signature · 1 line

2.10.0

Quoted 9 lines · Kees Cook

Quoted 8 lines · Hans Liljestrand

Quoted 6 lines · Kees Cook

Quoted 1 line · Elena

Thank you Kees! I applied the commit to our hardened_atomic_on_next branch and it will be included into the next rfc.

Cool, thanks. I assume this should get atomic64_t and local_t tests as well?

Quoted 7 lines

Quoted 3 lines

Yes, I'm currently compiling a build with atomic64_t and local_t tests added. With the improved lkdtm macros its much easier to add the extra types, thank you Kees!

Sure thing! I may have yet-another patch for this, as I didn't like repeating the same things in three files whenever a new test was added. Moar macro magick!

It would be nice to expose atomic* variables to userspace via debugfs so that we can confirm that the values will not be changed if overflowed. See the attached patch.

We will be able to check the test result:

 # /bin/echo ATOMIC_ADD_OVERFLOW > /debug/provoke-crash/DIRECT
 # echo $?
 # if [ cat /debug/provoke-crash/atomic -eq INT_MAX ]; then
 # echo PASS ; fi

Thanks,
-Takahiro AKASHI

Quoted 6 lines

-Kees

--
Kees Cook
Nexus Security

===8<===
From c516b50b4764c5c1ba0dd39e3a5022d026e35514 Mon Sep 17 00:00:00 2001
From: AKASHI Takahiro <[email protected]>
Date: Fri, 28 Oct 2016 10:55:50 +0900
Subject: [PATCH] lkdtm: expose atomic variables via debugfs

Signed-off-by AKASHI Takahiro <[email protected]>

---
 drivers/misc/lkdtm.h      |  2 ++
 drivers/misc/lkdtm_bugs.c | 75 +++++++++++++++++++++++++++++++++++++----------
 drivers/misc/lkdtm_core.c |  3 ++
 3 files changed, 64 insertions(+), 16 deletions(-)

drivers/misc/lkdtm.h +2 -0

index 0ef66ff..b4dd231 100644
@@ -3,10 +3,12 @@
 
 #define pr_fmt(fmt) "lkdtm: " fmt
 
+#include <linux/fs.h>
 #include <linux/kernel.h>
 
 /* lkdtm_bugs.c */
 void __init lkdtm_bugs_init(int *recur_param);
+void __init lkdtm_bugs_init2(struct dentry *parent);
 void lkdtm_PANIC(void);
 void lkdtm_BUG(void);
 void lkdtm_WARNING(void);

drivers/misc/lkdtm_bugs.c +2 -0

index 7b4067b..dd5003f 100644
@@ -5,6 +5,8 @@
  * test source files.
  */
 #include "lkdtm.h"
+#include <linux/debugfs.h>
+#include <linux/fs.h>
 #include <linux/sched.h>
 
 /*