From 67af976c806994cec27414d24b43f6519d72c240 Mon Sep 17 00:00:00 2001 From: Mr NeilBrown Date: Tue, 9 Feb 2021 11:49:30 +1100 Subject: [PATCH] LU-14428 libcfs: discard cfs_trace_copyin_string() 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. Signed-off-by: Mr NeilBrown Change-Id: I089c5da96b59ec62d177aea2f3d170bf751c6fec Reviewed-on: https://review.whamcloud.com/41490 Tested-by: jenkins Tested-by: Maloo Reviewed-by: James Simmons Reviewed-by: Arshad Hussain Reviewed-by: Oleg Drokin --- libcfs/include/libcfs/libcfs_debug.h | 2 - libcfs/libcfs/module.c | 19 ++++----- libcfs/libcfs/tracefile.c | 76 ++++++++++-------------------------- libcfs/libcfs/tracefile.h | 2 - lnet/lnet/router_proc.c | 24 ++++++------ 5 files changed, 42 insertions(+), 81 deletions(-) diff --git a/libcfs/include/libcfs/libcfs_debug.h b/libcfs/include/libcfs/libcfs_debug.h index cfbfafe..119e450 100644 --- a/libcfs/include/libcfs/libcfs_debug.h +++ b/libcfs/include/libcfs/libcfs_debug.h @@ -291,8 +291,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/libcfs/libcfs/module.c b/libcfs/libcfs/module.c index 1e5f6f1..d7c1bcf 100644 --- a/libcfs/libcfs/module.c +++ b/libcfs/libcfs/module.c @@ -287,17 +287,16 @@ static int __proc_dobitmasks(void *data, int write, loff_t pos, void __user *buffer, int nob) { const int tmpstrlen = 512; - char *tmpstr; + char *tmpstr = NULL; int rc; unsigned int *mask = data; int is_subsys = (mask == &libcfs_subsystem_debug) ? 1 : 0; int is_printk = (mask == &libcfs_printk) ? 1 : 0; - rc = cfs_trace_allocate_string_buffer(&tmpstr, tmpstrlen); - if (rc < 0) - return rc; - if (!write) { + rc = cfs_trace_allocate_string_buffer(&tmpstr, tmpstrlen); + if (rc < 0) + return rc; libcfs_debug_mask2str(tmpstr, tmpstrlen, *mask, is_subsys); rc = strlen(tmpstr); @@ -308,13 +307,11 @@ static int __proc_dobitmasks(void *data, 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/libcfs/libcfs/tracefile.c b/libcfs/libcfs/tracefile.c index 93c1fb3..36a16fa 100644 --- a/libcfs/libcfs/tracefile.c +++ b/libcfs/libcfs/tracefile.c @@ -873,33 +873,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(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) { @@ -939,26 +912,22 @@ int cfs_trace_allocate_string_buffer(char **str, int nob) int cfs_trace_dump_debug_buffer_usrstr(void __user *usr_str, int usr_str_nob) { - char *str; - int rc; + char *str; + char *path; + int rc; - rc = cfs_trace_allocate_string_buffer(&str, usr_str_nob + 1); - if (rc != 0) - return rc; + 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 != 0) - goto out; - - if (str[0] != '/') { - rc = -EINVAL; - goto out; - } - rc = cfs_tracefile_dump_all_pages(str); -out: + path = strim(str); + if (path[0] != '/') + rc = -EINVAL; + else + rc = cfs_tracefile_dump_all_pages(path); kfree(str); - return rc; + + return rc; } int cfs_trace_daemon_command(char *str) @@ -1002,20 +971,17 @@ int cfs_trace_daemon_command(char *str) int cfs_trace_daemon_command_usrstr(void __user *usr_str, int usr_str_nob) { - char *str; - int rc; - - rc = cfs_trace_allocate_string_buffer(&str, usr_str_nob + 1); - if (rc != 0) - return rc; + char *str; + int rc; - rc = cfs_trace_copyin_string(str, usr_str_nob + 1, - usr_str, usr_str_nob); - if (rc == 0) - rc = cfs_trace_daemon_command(str); + str = memdup_user_nul(usr_str, usr_str_nob); + if (!str) + return -ENOMEM; + rc = cfs_trace_daemon_command(strim(str)); kfree(str); - return rc; + + return rc; } int cfs_trace_set_debug_mb(int mb) diff --git a/libcfs/libcfs/tracefile.h b/libcfs/libcfs/tracefile.h index 8413413..601442d 100644 --- a/libcfs/libcfs/tracefile.h +++ b/libcfs/libcfs/tracefile.h @@ -54,8 +54,6 @@ 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_allocate_string_buffer(char **str, int nob); diff --git a/lnet/lnet/router_proc.c b/lnet/lnet/router_proc.c index 4a5952e7..c99454c 100644 --- a/lnet/lnet/router_proc.c +++ b/lnet/lnet/router_proc.c @@ -796,11 +796,11 @@ static int __proc_lnet_portal_rotor(void *data, int write, int rc; int i; - LIBCFS_ALLOC(buf, buf_len); - if (buf == NULL) - return -ENOMEM; - if (!write) { + LIBCFS_ALLOC(buf, buf_len); + if (buf == NULL) + return -ENOMEM; + lnet_res_lock(0); for (i = 0; portal_rotors[i].pr_value >= 0; i++) { @@ -821,14 +821,16 @@ static int __proc_lnet_portal_rotor(void *data, int write, rc = 0; } else { rc = cfs_trace_copyout_string(buffer, nob, - buf + pos, "\n"); + buf + pos, "\n"); } - goto out; + LIBCFS_FREE(buf, buf_len); + + 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); @@ -843,8 +845,8 @@ static int __proc_lnet_portal_rotor(void *data, int write, } } lnet_res_unlock(0); -out: - LIBCFS_FREE(buf, buf_len); + kfree(buf); + return rc; } -- 1.8.3.1