[PATCH] module: make perm const, change BUG_ON to BUILD_BUG_ON

From: Dan Streetman <ddstreet@ieee.org>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: Jani Nikula <jani.nikula@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"David S. Miller" <davem@davemloft.net>,
	Christoph Hellwig <hch@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Dan Streetman <ddstreet@ieee.org>
Subject: [PATCH] module: make perm const, change BUG_ON to BUILD_BUG_ON
Date: Wed, 10 Jun 2015 13:44:14 -0400	[thread overview]
Message-ID: <1433958254-6776-1-git-send-email-ddstreet@ieee.org> (raw)
In-Reply-To: <CALZtONDYaw6+9R_aebnyz3h_FO=+zAD3uB3cn5oUiEbQLjaQxA@mail.gmail.com>

Change the struct kernel_param.perm field to a const, as it should never
be changed.  Add inline functions that return a const value, which are
compiled out, for kparam_[un]block_sysfs_r/w() macros to use; and change
the BUG_ON() in those macros to BUILD_BUG_ON().

This will enforce that the kernel_param permissions are never changed,
and it will fail at build instead of runtime for any misuse of
sysfs param blocking.

I initially wanted to just use the const param.perm in the block_sysfs
checks, but unfortunately that also requires the param itself to be
const, in order to do const folding into a constant compiletime
condition to BUILD_BUG_ON(), and on some archs the params aren't defined
as const:

/* On alpha, ia64 and ppc64 relocations to global data cannot go into
   read-only sections (which is part of respective UNIX ABI on these
   platforms). So 'const' makes no sense and even causes compile failures
   with some compilers. */

So instead, this copies the idea from __param_check() to add inline
constant functions to check the param permissions.  That way no extra
bits are needed.

Signed-off-by: Dan Streetman <ddstreet@ieee.org>
---
 include/linux/moduleparam.h | 41 +++++++++++++++++++++++------------------
 kernel/params.c             |  8 +++-----
 2 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 1c9effa..032f1ce 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -68,7 +68,7 @@ enum {
 struct kernel_param {
 	const char *name;
 	const struct kernel_param_ops *ops;
-	u16 perm;
+	const u16 perm;
 	s8 level;
 	u8 flags;
 	union {
@@ -215,8 +215,13 @@ struct kparam_array
 /* This is the fundamental function for registering boot/module
    parameters. */
 #define __module_param_call(prefix, name, ops, arg, perm, level, flags)	\
+	/* These allow kparam_block_sysfs_*() to use BUILD_BUG_ON() */	\
+	static __attribute__((unused)) __always_inline const u16	\
+	__param_perm_rcheck_##name(void) { return (perm) & 0444; };	\
+	static __attribute__((unused)) __always_inline const u16	\
+	__param_perm_wcheck_##name(void) { return (perm) & 0222; };	\
 	/* Default value instead of permissions? */			\
-	static const char __param_str_##name[] = prefix #name; \
+	static const char __param_str_##name[] = prefix #name;		\
 	static struct kernel_param __moduleparam_const __param_##name	\
 	__used								\
     __attribute__ ((unused,__section__ ("__param"),aligned(sizeof(void *)))) \
@@ -244,20 +249,20 @@ __check_old_set_param(int (*oldset)(const char *, struct kernel_param *))
  *
  * There's no point blocking write on a paramter that isn't writable via sysfs!
  */
-#define kparam_block_sysfs_write(name)			\
-	do {						\
-		BUG_ON(!(__param_##name.perm & 0222));	\
-		__kernel_param_lock();			\
+#define kparam_block_sysfs_write(name)					\
+	do {								\
+		BUILD_BUG_ON(!__param_perm_wcheck_##name());		\
+		__kernel_param_lock();					\
 	} while (0)
 
 /**
  * kparam_unblock_sysfs_write - allows sysfs to write to a parameter again.
  * @name: the name of the parameter
  */
-#define kparam_unblock_sysfs_write(name)		\
-	do {						\
-		BUG_ON(!(__param_##name.perm & 0222));	\
-		__kernel_param_unlock();		\
+#define kparam_unblock_sysfs_write(name)				\
+	do {								\
+		BUILD_BUG_ON(!__param_perm_wcheck_##name());		\
+		__kernel_param_unlock();				\
 	} while (0)
 
 /**
@@ -266,20 +271,20 @@ __check_old_set_param(int (*oldset)(const char *, struct kernel_param *))
  *
  * This also blocks sysfs writes.
  */
-#define kparam_block_sysfs_read(name)			\
-	do {						\
-		BUG_ON(!(__param_##name.perm & 0444));	\
-		__kernel_param_lock();			\
+#define kparam_block_sysfs_read(name)					\
+	do {								\
+		BUILD_BUG_ON(!__param_perm_rcheck_##name());		\
+		__kernel_param_lock();					\
 	} while (0)
 
 /**
  * kparam_unblock_sysfs_read - allows sysfs to read a parameter again.
  * @name: the name of the parameter
  */
-#define kparam_unblock_sysfs_read(name)			\
-	do {						\
-		BUG_ON(!(__param_##name.perm & 0444));	\
-		__kernel_param_unlock();		\
+#define kparam_unblock_sysfs_read(name)					\
+	do {								\
+		BUILD_BUG_ON(!__param_perm_rcheck_##name());		\
+		__kernel_param_unlock();				\
 	} while (0)
 
 #ifdef CONFIG_SYSFS
diff --git a/kernel/params.c b/kernel/params.c
index a22d6a7..4e28ff1 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -364,12 +364,11 @@ EXPORT_SYMBOL(param_ops_invbool);
 
 int param_set_bint(const char *val, const struct kernel_param *kp)
 {
-	struct kernel_param boolkp;
+	/* Match bool exactly, by re-using it. */
+	struct kernel_param boolkp = *kp;
 	bool v;
 	int ret;
 
-	/* Match bool exactly, by re-using it. */
-	boolkp = *kp;
 	boolkp.arg = &v;
 
 	ret = param_set_bool(val, &boolkp);
@@ -449,9 +448,8 @@ static int param_array_get(char *buffer, const struct kernel_param *kp)
 {
 	int i, off, ret;
 	const struct kparam_array *arr = kp->arr;
-	struct kernel_param p;
+	struct kernel_param p = *kp;
 
-	p = *kp;
 	for (i = off = 0; i < (arr->num ? *arr->num : arr->max); i++) {
 		if (i)
 			buffer[off++] = ',';
-- 
2.1.0

