[lustre-devel] [PATCH 37/49] lnet: libcfs: discard cfs_trace_copyin_string()

[James Simmons <jsimmons@infradead.org>]

From: Mr NeilBrown <neilb@suse.de>

Instead of cfs_trace_copyin_string(), use memdup_user_nul().
This combines the allocation with the copyin, and nul-terminates.

The resulting code is a lot simpler.

WC-bug-id: https://jira.whamcloud.com/browse/LU-14428
Lustre-commit: 67af976c806994ce ("LU-14428 libcfs: discard cfs_trace_copyin_string()")
Signed-off-by: Mr NeilBrown <neilb@suse.de>
Reviewed-on: https://review.whamcloud.com/41490
Reviewed-by: James Simmons <jsimmons@infradead.org>
Reviewed-by: Arshad Hussain <arshad.hussain@aeoncomputing.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 include/linux/libcfs/libcfs_debug.h |  2 --
 net/lnet/libcfs/module.c            | 19 +++++-------
 net/lnet/libcfs/tracefile.c         | 59 +++++++------------------------------
 net/lnet/libcfs/tracefile.h         |  2 --
 net/lnet/lnet/router_proc.c         | 22 +++++++-------
 5 files changed, 30 insertions(+), 74 deletions(-)

diff --git a/include/linux/libcfs/libcfs_debug.h b/include/linux/libcfs/libcfs_debug.h
index 99905f7..93eb752 100644
--- a/include/linux/libcfs/libcfs_debug.h
+++ b/include/linux/libcfs/libcfs_debug.h
@@ -209,8 +209,6 @@ int libcfs_debug_msg(struct libcfs_debug_msg_data *msgdata,
 	__printf(2, 3);
 
 /* other external symbols that tracefile provides: */
-int cfs_trace_copyin_string(char *knl_buffer, int knl_buffer_nob,
-			    const char __user *usr_buffer, int usr_buffer_nob);
 int cfs_trace_copyout_string(char __user *usr_buffer, int usr_buffer_nob,
 			     const char *knl_buffer, char *append);
 
diff --git a/net/lnet/libcfs/module.c b/net/lnet/libcfs/module.c
index f9cc6df..93e9b9e 100644
--- a/net/lnet/libcfs/module.c
+++ b/net/lnet/libcfs/module.c
@@ -295,7 +295,7 @@ static int proc_dobitmasks(struct ctl_table *table, int write,
 			   void __user *buffer, size_t *lenp, loff_t *ppos)
 {
 	const int tmpstrlen = 512;
-	char *tmpstr;
+	char *tmpstr = NULL;
 	int rc;
 	size_t nob = *lenp;
 	loff_t pos = *ppos;
@@ -303,11 +303,10 @@ static int proc_dobitmasks(struct ctl_table *table, int write,
 	int is_subsys = (mask == &libcfs_subsystem_debug) ? 1 : 0;
 	int is_printk = (mask == &libcfs_printk) ? 1 : 0;
 
-	tmpstr = kzalloc(tmpstrlen, GFP_KERNEL);
-	if (!tmpstr)
-		return -ENOMEM;
-
 	if (!write) {
+		tmpstr = kzalloc(tmpstrlen, GFP_KERNEL);
+		if (!tmpstr)
+			return -ENOMEM;
 		libcfs_debug_mask2str(tmpstr, tmpstrlen, *mask, is_subsys);
 		rc = strlen(tmpstr);
 
@@ -318,13 +317,11 @@ static int proc_dobitmasks(struct ctl_table *table, int write,
 						      tmpstr + pos, "\n");
 		}
 	} else {
-		rc = cfs_trace_copyin_string(tmpstr, tmpstrlen, buffer, nob);
-		if (rc < 0) {
-			kfree(tmpstr);
-			return rc;
-		}
+		tmpstr = memdup_user_nul(buffer, nob);
+		if (!tmpstr)
+			return -ENOMEM;
 
-		rc = libcfs_debug_str2mask(mask, tmpstr, is_subsys);
+		rc = libcfs_debug_str2mask(mask, strim(tmpstr), is_subsys);
 		/* Always print LBUG/LASSERT to console, so keep this mask */
 		if (is_printk)
 			*mask |= D_EMERG;
diff --git a/net/lnet/libcfs/tracefile.c b/net/lnet/libcfs/tracefile.c
index e3a063f..731623b 100644
--- a/net/lnet/libcfs/tracefile.c
+++ b/net/lnet/libcfs/tracefile.c
@@ -920,34 +920,6 @@ void cfs_trace_flush_pages(void)
 	}
 }
 
-int cfs_trace_copyin_string(char *knl_buffer, int knl_buffer_nob,
-			    const char __user *usr_buffer, int usr_buffer_nob)
-{
-	int nob;
-
-	if (usr_buffer_nob > knl_buffer_nob)
-		return -EOVERFLOW;
-
-	if (copy_from_user((void *)knl_buffer,
-			   usr_buffer, usr_buffer_nob))
-		return -EFAULT;
-
-	nob = strnlen(knl_buffer, usr_buffer_nob);
-	while (--nob >= 0)		/* strip trailing whitespace */
-		if (!isspace(knl_buffer[nob]))
-			break;
-
-	if (nob < 0)			/* empty string */
-		return -EINVAL;
-
-	if (nob == knl_buffer_nob)	/* no space to terminate */
-		return -EOVERFLOW;
-
-	knl_buffer[nob + 1] = 0;	/* terminate */
-	return 0;
-}
-EXPORT_SYMBOL(cfs_trace_copyin_string);
-
 int cfs_trace_copyout_string(char __user *usr_buffer, int usr_buffer_nob,
 			     const char *knl_buffer, char *append)
 {
@@ -977,26 +949,20 @@ int cfs_trace_copyout_string(char __user *usr_buffer, int usr_buffer_nob,
 int cfs_trace_dump_debug_buffer_usrstr(void __user *usr_str, int usr_str_nob)
 {
 	char *str;
+	char *path;
 	int rc;
 
-	if (usr_str_nob >= 2 * PAGE_SIZE)
-		return -EINVAL;
-	str = kzalloc(usr_str_nob + 1, GFP_KERNEL);
+	str = memdup_user_nul(usr_str, usr_str_nob);
 	if (!str)
 		return -ENOMEM;
 
-	rc = cfs_trace_copyin_string(str, usr_str_nob + 1,
-				     usr_str, usr_str_nob);
-	if (rc)
-		goto out;
-
-	if (str[0] != '/') {
+	path = strim(str);
+	if (path[0] != '/')
 		rc = -EINVAL;
-		goto out;
-	}
-	rc = cfs_tracefile_dump_all_pages(str);
-out:
+	else
+		rc = cfs_tracefile_dump_all_pages(str);
 	kfree(str);
+
 	return rc;
 }
 
@@ -1045,18 +1011,13 @@ int cfs_trace_daemon_command_usrstr(void __user *usr_str, int usr_str_nob)
 	char *str;
 	int rc;
 
-	if (usr_str_nob >= 2 * PAGE_SIZE)
-		return -EINVAL;
-	str = kzalloc(usr_str_nob + 1, GFP_KERNEL);
+	str = memdup_user_nul(usr_str, usr_str_nob);
 	if (!str)
 		return -ENOMEM;
 
-	rc = cfs_trace_copyin_string(str, usr_str_nob + 1,
-				     usr_str, usr_str_nob);
-	if (!rc)
-		rc = cfs_trace_daemon_command(str);
-
+	rc = cfs_trace_daemon_command(str);
 	kfree(str);
+
 	return rc;
 }
 
diff --git a/net/lnet/libcfs/tracefile.h b/net/lnet/libcfs/tracefile.h
index 5b90c1b..311ec8c 100644
--- a/net/lnet/libcfs/tracefile.h
+++ b/net/lnet/libcfs/tracefile.h
@@ -59,8 +59,6 @@
 int cfs_tracefile_init(int max_pages);
 void cfs_tracefile_exit(void);
 
-int cfs_trace_copyin_string(char *knl_buffer, int knl_buffer_nob,
-			    const char __user *usr_buffer, int usr_buffer_nob);
 int cfs_trace_copyout_string(char __user *usr_buffer, int usr_buffer_nob,
 			     const char *knl_str, char *append);
 int cfs_trace_dump_debug_buffer_usrstr(void __user *usr_str, int usr_str_nob);
diff --git a/net/lnet/lnet/router_proc.c b/net/lnet/lnet/router_proc.c
index 623899e..25d172d 100644
--- a/net/lnet/lnet/router_proc.c
+++ b/net/lnet/lnet/router_proc.c
@@ -743,7 +743,7 @@ struct lnet_portal_rotors {
 	const char	*pr_desc;
 };
 
-static struct lnet_portal_rotors	portal_rotors[] = {
+static struct lnet_portal_rotors portal_rotors[] = {
 	{
 		.pr_value = LNET_PTL_ROTOR_OFF,
 		.pr_name  = "OFF",
@@ -783,11 +783,11 @@ static int proc_lnet_portal_rotor(struct ctl_table *table, int write,
 	int rc;
 	int i;
 
-	buf = kmalloc(buf_len, GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
-
 	if (!write) {
+		buf = kmalloc(buf_len, GFP_KERNEL);
+		if (!buf)
+			return -ENOMEM;
+
 		lnet_res_lock(0);
 
 		for (i = 0; portal_rotors[i].pr_value >= 0; i++) {
@@ -810,12 +810,14 @@ static int proc_lnet_portal_rotor(struct ctl_table *table, int write,
 			rc = cfs_trace_copyout_string(buffer, nob,
 						      buf + pos, "\n");
 		}
-		goto out;
+		kfree(buf);
+
+		return rc;
 	}
 
-	rc = cfs_trace_copyin_string(buf, buf_len, buffer, nob);
-	if (rc < 0)
-		goto out;
+	buf = memdup_user_nul(buffer, nob);
+	if (!buf)
+		return -ENOMEM;
 
 	tmp = strim(buf);
 
@@ -830,8 +832,8 @@ static int proc_lnet_portal_rotor(struct ctl_table *table, int write,
 		}
 	}
 	lnet_res_unlock(0);
-out:
 	kfree(buf);
+
 	return rc;
 }
 
-- 
1.8.3.1

_______________________________________________
lustre-devel mailing list
lustre-devel@lists.lustre.org
http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org