All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nathan Fontenot <nfont@linux.vnet.ibm.com>
To: linuxppc-dev@lists.ozlabs.org
Cc: sahilmehta17@gmail.com
Subject: [PATCH v6 1/3] powerpc/pseries: Correct possible read beyond dlpar sysfs buffer
Date: Tue, 18 Oct 2016 13:20:26 -0400	[thread overview]
Message-ID: <20161018172012.37287.76194.stgit@ltcalpine2-lp14.aus.stglabs.ibm.com> (raw)
In-Reply-To: <20161018171803.37287.54834.stgit@ltcalpine2-lp14.aus.stglabs.ibm.com>

The pasrsing of data written to the dlpar file in sysfs does not correctly
account for the possibility of reading past the end of the buffer. Correct
this by updating the buffer parsing code to make a local copy and use the
strsep() and sysfs_streq() routines to parse the buffer. This also
separates the parsing code into subroutines to make cleaner.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/dlpar.c |  141 ++++++++++++++++++++++----------
 1 file changed, 96 insertions(+), 45 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index 423e450..ec5d677 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -418,84 +418,135 @@ void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
 	}
 }
 
-static ssize_t dlpar_store(struct class *class, struct class_attribute *attr,
-			   const char *buf, size_t count)
+static int dlpar_parse_resource(char **cmd, struct pseries_hp_errorlog *hp_elog)
 {
-	struct pseries_hp_errorlog *hp_elog;
-	struct completion hotplug_done;
-	const char *arg;
-	int rc;
+	char *arg;
 
-	hp_elog = kzalloc(sizeof(*hp_elog), GFP_KERNEL);
-	if (!hp_elog) {
-		rc = -ENOMEM;
-		goto dlpar_store_out;
-	}
+	arg = strsep(cmd, " ");
+	if (!arg)
+		return -EINVAL;
 
-	/* Parse out the request from the user, this will be in the form
-	 * <resource> <action> <id_type> <id>
-	 */
-	arg = buf;
-	if (!strncmp(arg, "memory", 6)) {
+	if (sysfs_streq(arg, "memory")) {
 		hp_elog->resource = PSERIES_HP_ELOG_RESOURCE_MEM;
-		arg += strlen("memory ");
-	} else if (!strncmp(arg, "cpu", 3)) {
+	} else if (sysfs_streq(arg, "cpu")) {
 		hp_elog->resource = PSERIES_HP_ELOG_RESOURCE_CPU;
-		arg += strlen("cpu ");
 	} else {
-		pr_err("Invalid resource specified: \"%s\"\n", buf);
-		rc = -EINVAL;
-		goto dlpar_store_out;
+		pr_err("Invalid resource specified.\n");
+		return -EINVAL;
 	}
 
-	if (!strncmp(arg, "add", 3)) {
+	return 0;
+}
+
+static int dlpar_parse_action(char **cmd, struct pseries_hp_errorlog *hp_elog)
+{
+	char *arg;
+
+	arg = strsep(cmd, " ");
+	if (!arg)
+		return -EINVAL;
+
+	if (sysfs_streq(arg, "add")) {
 		hp_elog->action = PSERIES_HP_ELOG_ACTION_ADD;
-		arg += strlen("add ");
-	} else if (!strncmp(arg, "remove", 6)) {
+	} else if (sysfs_streq(arg, "remove")) {
 		hp_elog->action = PSERIES_HP_ELOG_ACTION_REMOVE;
-		arg += strlen("remove ");
 	} else {
-		pr_err("Invalid action specified: \"%s\"\n", buf);
-		rc = -EINVAL;
-		goto dlpar_store_out;
+		pr_err("Invalid action specified.\n");
+		return -EINVAL;
 	}
 
-	if (!strncmp(arg, "index", 5)) {
-		u32 index;
+	return 0;
+}
 
+static int dlpar_parse_id_type(char **cmd, struct pseries_hp_errorlog *hp_elog)
+{
+	char *arg;
+	u32 count, index;
+
+	arg = strsep(cmd, " ");
+	if (!arg)
+		return -EINVAL;
+
+	if (sysfs_streq(arg, "index")) {
 		hp_elog->id_type = PSERIES_HP_ELOG_ID_DRC_INDEX;
-		arg += strlen("index ");
+		arg = strsep(cmd, " ");
+		if (!arg) {
+			pr_err("No DRC Index specified.\n");
+			return -EINVAL;
+		}
+
 		if (kstrtou32(arg, 0, &index)) {
-			rc = -EINVAL;
-			pr_err("Invalid drc_index specified: \"%s\"\n", buf);
-			goto dlpar_store_out;
+			pr_err("Invalid DRC Index specified.\n");
+			return -EINVAL;
 		}
 
 		hp_elog->_drc_u.drc_index = cpu_to_be32(index);
-	} else if (!strncmp(arg, "count", 5)) {
-		u32 count;
-
+	} else if (sysfs_streq(arg, "count")) {
 		hp_elog->id_type = PSERIES_HP_ELOG_ID_DRC_COUNT;
-		arg += strlen("count ");
+		arg = strsep(cmd, " ");
+		if (!arg) {
+			pr_err("No DRC count specified.\n");
+			return -EINVAL;
+		}
+
 		if (kstrtou32(arg, 0, &count)) {
-			rc = -EINVAL;
-			pr_err("Invalid count specified: \"%s\"\n", buf);
-			goto dlpar_store_out;
+			pr_err("Invalid DRC count specified.\n");
+			return -EINVAL;
 		}
 
 		hp_elog->_drc_u.drc_count = cpu_to_be32(count);
 	} else {
-		pr_err("Invalid id_type specified: \"%s\"\n", buf);
-		rc = -EINVAL;
-		goto dlpar_store_out;
+		pr_err("Invalid id_type specified.\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static ssize_t dlpar_store(struct class *class, struct class_attribute *attr,
+			   const char *buf, size_t count)
+{
+	struct pseries_hp_errorlog *hp_elog;
+	struct completion hotplug_done;
+	char *argbuf;
+	char *args;
+	int rc;
+
+	args = argbuf = kstrdup(buf, GFP_KERNEL);
+	hp_elog = kzalloc(sizeof(*hp_elog), GFP_KERNEL);
+	if (!hp_elog || !argbuf) {
+		pr_info("Could not allocate resources for DLPAR operation\n");
+		kfree(argbuf);
+		kfree(hp_elog);
+		return -ENOMEM;
 	}
 
+	/* Parse out the request from the user, this will be in the form
+	 * <resource> <action> <id_type> <id>
+	 */
+	rc = dlpar_parse_resource(&args, hp_elog);
+	if (rc)
+		goto dlpar_store_out;
+
+	rc = dlpar_parse_action(&args, hp_elog);
+	if (rc)
+		goto dlpar_store_out;
+
+	rc = dlpar_parse_id_type(&args, hp_elog);
+	if (rc)
+		goto dlpar_store_out;
+
 	init_completion(&hotplug_done);
 	queue_hotplug_event(hp_elog, &hotplug_done, &rc);
 	wait_for_completion(&hotplug_done);
 
 dlpar_store_out:
+	kfree(argbuf);
 	kfree(hp_elog);
+
+	if (rc)
+		pr_err("Could not handle DLPAR request \"%s\"\n", buf);
+
 	return rc ? rc : count;
 }
 

  reply	other threads:[~2016-10-18 17:28 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-18 17:20 [PATCH v6 0/3] powerpc/pseries: Implement indexed-count memory hotplug Nathan Fontenot
2016-10-18 17:20 ` Nathan Fontenot [this message]
2016-10-27 14:20   ` [PATCH v6 1/3] powerpc/pseries: Correct possible read beyond dlpar sysfs buffer John Allen
2016-10-18 17:20 ` [PATCH v6 2/3] powerpc/pseries: Implement indexed-count hotplug memory add Nathan Fontenot
2016-10-27 14:21   ` John Allen
2016-10-18 17:21 ` [PATCH v6 3/3] powerpc/pseries: Implement indexed-count hotplug memory remove Nathan Fontenot
2016-10-27 14:22   ` John Allen
2016-11-15  0:58   ` Michael Roth
2016-11-15 20:09     ` Nathan Fontenot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161018172012.37287.76194.stgit@ltcalpine2-lp14.aus.stglabs.ibm.com \
    --to=nfont@linux.vnet.ibm.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=sahilmehta17@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.