Re: [PATCH V38 19/29] Lock down module params that specify hardware parameters (eg. ioport)

From: David Howells <dhowells@redhat.com>

Provided an annotation for module parameters that specify hardware
parameters (such as io ports, iomem addresses, irqs, dma channels, fixed
dma buffers and other types).

Suggested-by: Alan Cox <redacted>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Matthew Garrett <redacted>
Reviewed-by: Kees Cook <redacted>
Cc: Jessica Yu <jeyu@kernel.org>
---
include/linux/security.h     |  1 +
kernel/params.c              | 27 ++++++++++++++++++++++-----
security/lockdown/lockdown.c |  1 +
3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/include/linux/security.h b/include/linux/security.h
index 8f7048395114..43fa3486522b 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h

@@ -113,6 +113,7 @@ enum lockdown_reason {

	LOCKDOWN_ACPI_TABLES,
	LOCKDOWN_PCMCIA_CIS,
	LOCKDOWN_TIOCSSERIAL,
+	LOCKDOWN_MODULE_PARAMETERS,
	LOCKDOWN_INTEGRITY_MAX,
	LOCKDOWN_CONFIDENTIALITY_MAX,
};

diff --git a/kernel/params.c b/kernel/params.c
index cf448785d058..35f138fce762 100644
--- a/kernel/params.c
+++ b/kernel/params.c

@@ -12,6 +12,7 @@

#include <linux/err.h>
#include <linux/slab.h>
#include <linux/ctype.h>
+#include <linux/security.h>

#ifdef CONFIG_SYSFS
/* Protects all built-in parameters, modules use their own param_lock */

@@ -96,13 +97,19 @@ bool parameq(const char *a, const char *b)

	return parameqn(a, b, strlen(a)+1);
}

-static void param_check_unsafe(const struct kernel_param *kp)
+static bool param_check_unsafe(const struct kernel_param *kp)
{
+	if (kp->flags & KERNEL_PARAM_FL_HWPARAM &&
+	    security_locked_down(LOCKDOWN_MODULE_PARAMETERS))
+		return false;
+
	if (kp->flags & KERNEL_PARAM_FL_UNSAFE) {
		pr_notice("Setting dangerous option %s - tainting kernel\n",
			  kp->name);
		add_taint(TAINT_USER, LOCKDEP_STILL_OK);
	}
+
+	return true;
}

static int parse_one(char *param,

@@ -132,8 +139,10 @@ static int parse_one(char *param,

			pr_debug("handling %s with %p\n", param,
				params[i].ops->set);
			kernel_param_lock(params[i].mod);
-			param_check_unsafe(&params[i]);
-			err = params[i].ops->set(val, &params[i]);
+			if (param_check_unsafe(&params[i]))
+				err = params[i].ops->set(val, &params[i]);
+			else
+				err = -EPERM;
			kernel_param_unlock(params[i].mod);
			return err;
		}

@@ -541,6 +550,12 @@ static ssize_t param_attr_show(struct module_attribute *mattr,

	return count;
}

+#ifdef CONFIG_MODULES
+#define mod_name(mod) ((mod)->name)
+#else
+#define mod_name(mod) "unknown"
+#endif
+

/* sysfs always hands a nul-terminated string in buf.  We rely on that. */
static ssize_t param_attr_store(struct module_attribute *mattr,
				struct module_kobject *mk,

@@ -553,8 +568,10 @@ static ssize_t param_attr_store(struct module_attribute *mattr,

		return -EPERM;

	kernel_param_lock(mk->mod);
-	param_check_unsafe(attribute->param);
-	err = attribute->param->ops->set(buf, attribute->param);
+	if (param_check_unsafe(attribute->param))
+		err = attribute->param->ops->set(buf, attribute->param);
+	else
+		err = -EPERM;
	kernel_param_unlock(mk->mod);
	if (!err)
		return len;

diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 00a3a6438dd2..5177938cfa0d 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c

@@ -28,6 +28,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {

	[LOCKDOWN_ACPI_TABLES] = "modifying ACPI tables",
	[LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
	[LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO",
+	[LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
	[LOCKDOWN_INTEGRITY_MAX] = "integrity",
	[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
-- 
2.22.0.770.g0f2c4a37fd-goog