linux-kernel - [RESEND PATCH] module param_call: fix potential NULL pointer dereference

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <1266835251-15457-1-git-send-email-dongdong.deng@windriver.com>
Date:	Mon, 22 Feb 2010 18:40:51 +0800
From:	Dongdong Deng <dongdong.deng@...driver.com>
To:	rusty@...tcorp.com.au, xiyou.wangcong@...il.com
Cc:	linux-kernel@...r.kernel.org, jason.wessel@...driver.com,
	davem@...emloft.net, dongdong.deng@...driver.com
Subject: [RESEND PATCH] module param_call: fix potential NULL pointer dereference

The param_set_fn() function will get a parameter which is a NULL
pointer when insmod module via bare params as following method:

$insmod foo.ko foo

If the param_set_fn() function didn't check that parameter and used
it directly, it could caused an OOPS due to NULL pointer dereference.

The solution is simple:
Using "" to replace NULL parameter, thereby the param_set_fn()
function will never get a NULL pointer.

Signed-off-by: Dongdong Deng <dongdong.deng@...driver.com>
---
 kernel/params.c |   30 ++++++------------------------
 1 files changed, 6 insertions(+), 24 deletions(-)

diff --git a/kernel/params.c b/kernel/params.c
index cf1b691..548d680 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -101,7 +101,11 @@ static char *next_arg(char *args, char **param, char **val)
 
 	*param = args;
 	if (!equals)
-		*val = NULL;
+		/*
+		 * We used to hand NULL for bare params, but most code
+		 *  didn't handle it. Using "" to replace NULL here.
+		 */
+		*val = "";
 	else {
 		args[equals] = '\0';
 		*val = args + equals + 1;
@@ -180,10 +184,7 @@ int parse_args(const char *name,
 	int param_set_##name(const char *val, struct kernel_param *kp)	\
 	{								\
 		tmptype l;						\
-		int ret;						\
-									\
-		if (!val) return -EINVAL;				\
-		ret = strtolfn(val, 0, &l);				\
+		int ret = strtolfn(val, 0, &l);				\
 		if (ret == -EINVAL || ((type)l != l))			\
 			return -EINVAL;					\
 		*((type *)kp->arg) = l;					\
@@ -204,12 +205,6 @@ STANDARD_PARAM_DEF(ulong, unsigned long, "%lu", unsigned long, strict_strtoul);
 
 int param_set_charp(const char *val, struct kernel_param *kp)
 {
-	if (!val) {
-		printk(KERN_ERR "%s: string parameter expected\n",
-		       kp->name);
-		return -EINVAL;
-	}
-
 	if (strlen(val) > 1024) {
 		printk(KERN_ERR "%s: string parameter too long\n",
 		       kp->name);
@@ -238,9 +233,6 @@ int param_set_bool(const char *val, struct kernel_param *kp)
 {
 	bool v;
 
-	/* No equals means "set"... */
-	if (!val) val = "1";
-
 	/* One of =[yYnN01] */
 	switch (val[0]) {
 	case 'y': case 'Y': case '1':
@@ -310,12 +302,6 @@ static int param_array(const char *name,
 	kp.arg = elem;
 	kp.flags = flags;
 
-	/* No equals sign? */
-	if (!val) {
-		printk(KERN_ERR "%s: expects arguments\n", name);
-		return -EINVAL;
-	}
-
 	*num = 0;
 	/* We expect a comma-separated list of values. */
 	do {
@@ -382,10 +368,6 @@ int param_set_copystring(const char *val, struct kernel_param *kp)
 {
 	const struct kparam_string *kps = kp->str;
 
-	if (!val) {
-		printk(KERN_ERR "%s: missing param set value\n", kp->name);
-		return -EINVAL;
-	}
 	if (strlen(val)+1 > kps->maxlen) {
 		printk(KERN_ERR "%s: string doesn't fit in %u chars.\n",
 		       kp->name, kps->maxlen-1);
-- 
1.6.0.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/