linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] staging: wilc1000: Process WARN, INFO options of debug levels from user
@ 2015-08-12 17:35 Chandra S Gorentla
  2015-08-13 19:08 ` Dan Carpenter
  0 siblings, 1 reply; 2+ messages in thread
From: Chandra S Gorentla @ 2015-08-12 17:35 UTC (permalink / raw)
  To: gregkh
  Cc: johnny.kim, rachel.kim, dean.lee, chris.park, linux-wireless,
	devel, linux-kernel, dan.carpenter, Chandra S Gorentla

This patch enables setting the module's debug options WARN and INFO in the
debugfs file 'wilc_debug_level'.  This enables the user to enable logging
of warning and other information.  Before this change writes to this debugfs
file sets only one option DGB.  This is additional to the default option
ERR.

As a side effect, this patch removes the 'sparse' warning -
'warning: incorrect type in argument 2 (different address spaces)'.

Signed-off-by: Chandra S Gorentla <csgorentla@gmail.com>
---
 drivers/staging/wilc1000/wilc_debugfs.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_debugfs.c b/drivers/staging/wilc1000/wilc_debugfs.c
index be2e901..3934e8b 100644
--- a/drivers/staging/wilc1000/wilc_debugfs.c
+++ b/drivers/staging/wilc1000/wilc_debugfs.c
@@ -48,29 +48,26 @@ static ssize_t wilc_debug_level_read(struct file *file, char __user *userbuf, si
 	return simple_read_from_buffer(userbuf, count, ppos, buf, res);
 }
 
-static ssize_t wilc_debug_level_write(struct file *filp, const char *buf, size_t count, loff_t *ppos)
+static ssize_t wilc_debug_level_write(struct file *filp, const char __user *buf,
+					size_t count, loff_t *ppos)
 {
-	char buffer[128] = {};
+	char buffer[2] = {};  /* One character for 'level' + one for null */
 	int flag = 0;
+	int ret;
 
 	if (count > sizeof(buffer))
 		return -EINVAL;
 
-	if (copy_from_user(buffer, buf, count)) {
+	ret = simple_write_to_buffer(buffer, sizeof(buffer), ppos, buf, count);
+	if (ret != count)
 		return -EFAULT;
-	}
-
-	flag = buffer[0] - '0';
 
-	if (flag > 0)
-		flag = DEBUG | ERR;
-	else if (flag < 0)
-		flag = 100;
+	ret = kstrtoint(buffer, 16, &flag);
+	if (ret)
+		return -EINVAL;
 
-	if (flag > DBG_LEVEL_ALL) {
-		printk("%s, value (0x%08x) is out of range, stay previous flag (0x%08x)\n", __func__, flag, atomic_read(&DEBUG_LEVEL));
-		return -EFAULT;
-	}
+	if (flag > DBG_LEVEL_ALL)
+		return -EINVAL;
 
 	atomic_set(&DEBUG_LEVEL, (int)flag);
 
@@ -78,6 +75,7 @@ static ssize_t wilc_debug_level_write(struct file *filp, const char *buf, size_t
 		printk("Debug-level disabled\n");
 	else
 		printk("Debug-level enabled\n");
+
 	return count;
 }
 
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] staging: wilc1000: Process WARN, INFO options of debug levels from user
  2015-08-12 17:35 [PATCH] staging: wilc1000: Process WARN, INFO options of debug levels from user Chandra S Gorentla
@ 2015-08-13 19:08 ` Dan Carpenter
  0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2015-08-13 19:08 UTC (permalink / raw)
  To: Chandra S Gorentla
  Cc: gregkh, johnny.kim, rachel.kim, dean.lee, chris.park,
	linux-wireless, devel, linux-kernel

On Wed, Aug 12, 2015 at 11:05:58PM +0530, Chandra S Gorentla wrote:
> -	if (copy_from_user(buffer, buf, count)) {
> +	ret = simple_write_to_buffer(buffer, sizeof(buffer), ppos, buf, count);

This part doesn't make sense.  Use copy_from_user().  Also it's not NUL
terminated so it leads to a read past the end of the array later.  In
the original code, we just looked at the first char and didn't use
kstrtoint() so we didn't care about NUL termination.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2015-08-13 19:08 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-12 17:35 [PATCH] staging: wilc1000: Process WARN, INFO options of debug levels from user Chandra S Gorentla
2015-08-13 19:08 ` Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).