All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] lib: add "on" and "off" to strtobool
@ 2016-01-28 14:17 ` Kees Cook
  0 siblings, 0 replies; 14+ messages in thread
From: Kees Cook @ 2016-01-28 14:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Joe Perches, Rasmus Villemoes, Daniel Borkmann,
	Amitkumar Karwar, Nishant Sarmukadam, Kalle Valo, Steve French,
	Michael Ellerman, Heiko Carstens, Martin Schwidefsky, x86,
	linuxppc-dev, linux-s390, linux-wireless, netdev, linux-cifs,
	linux-kernel

This consolidates logic for handling "on"/"off" parsing for bools into
the existing strtobool function. This requires making sure callers are
passing NULL-terminated strings.

-Kees


^ permalink raw reply	[flat|nested] 14+ messages in thread
* [PATCH 0/3] lib: add "on" and "off" to strtobool
@ 2016-01-28 14:17 ` Kees Cook
  0 siblings, 0 replies; 14+ messages in thread
From: Kees Cook @ 2016-01-28 14:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Joe Perches, Rasmus Villemoes, Daniel Borkmann,
	Amitkumar Karwar, Nishant Sarmukadam, Kalle Valo, Steve French,
	Michael Ellerman, Heiko Carstens, Martin Schwidefsky,
	x86-DgEjT+Ai2ygdnm+yROfE0A, linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-s390-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

This consolidates logic for handling "on"/"off" parsing for bools into
the existing strtobool function. This requires making sure callers are
passing NULL-terminated strings.

-Kees

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread
* [PATCH 1/3] lib: fix callers of strtobool to use char array
@ 2016-01-28 14:17   ` Kees Cook
  0 siblings, 0 replies; 14+ messages in thread
From: Kees Cook @ 2016-01-28 14:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Amitkumar Karwar, Nishant Sarmukadam, Kalle Valo,
	Steve French, linux-cifs, Joe Perches, Rasmus Villemoes,
	Daniel Borkmann, Michael Ellerman, Heiko Carstens,
	Martin Schwidefsky, x86, linuxppc-dev, linux-s390,
	linux-wireless, netdev, linux-kernel

Some callers of strtobool were passing a pointer to unterminated strings.
This fixes the issue and consolidates some logic in cifs.

Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: Amitkumar Karwar <akarwar@marvell.com>
Cc: Nishant Sarmukadam <nishants@marvell.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: Steve French <sfrench@samba.org>
Cc: linux-cifs@vger.kernel.org
---
 drivers/net/wireless/marvell/mwifiex/debugfs.c |   6 +-
 fs/cifs/cifs_debug.c                           | 106 ++++++++++++-------------
 fs/cifs/cifs_debug.h                           |   2 +-
 fs/cifs/cifsfs.c                               |   6 +-
 fs/cifs/cifsglob.h                             |   4 +-
 5 files changed, 58 insertions(+), 66 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/debugfs.c b/drivers/net/wireless/marvell/mwifiex/debugfs.c
index 0b9c580af988..76af60899c69 100644
--- a/drivers/net/wireless/marvell/mwifiex/debugfs.c
+++ b/drivers/net/wireless/marvell/mwifiex/debugfs.c
@@ -880,13 +880,13 @@ mwifiex_reset_write(struct file *file,
 {
 	struct mwifiex_private *priv = file->private_data;
 	struct mwifiex_adapter *adapter = priv->adapter;
-	char cmd;
+	char cmd[2] = { '\0' };
 	bool result;
 
-	if (copy_from_user(&cmd, ubuf, sizeof(cmd)))
+	if (copy_from_user(cmd, ubuf, sizeof(char)))
 		return -EFAULT;
 
-	if (strtobool(&cmd, &result))
+	if (strtobool(cmd, &result))
 		return -EINVAL;
 
 	if (!result)
diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
index 50b268483302..2f7ffcc9e364 100644
--- a/fs/cifs/cifs_debug.c
+++ b/fs/cifs/cifs_debug.c
@@ -251,11 +251,29 @@ static const struct file_operations cifs_debug_data_proc_fops = {
 	.release	= single_release,
 };
 
+static int get_user_bool(const char __user *buffer, bool *store)
+{
+	char c[2] = { '\0' };
+	bool bv;
+	int rc;
+
+	rc = get_user(c[0], buffer);
+	if (rc)
+		return rc;
+
+	rc = strtobool(c, &bv);
+	if (rc)
+		return rc;
+
+	*store = bv;
+
+	return 0;
+}
+
 #ifdef CONFIG_CIFS_STATS
 static ssize_t cifs_stats_proc_write(struct file *file,
 		const char __user *buffer, size_t count, loff_t *ppos)
 {
-	char c;
 	bool bv;
 	int rc;
 	struct list_head *tmp1, *tmp2, *tmp3;
@@ -263,34 +281,32 @@ static ssize_t cifs_stats_proc_write(struct file *file,
 	struct cifs_ses *ses;
 	struct cifs_tcon *tcon;
 
-	rc = get_user(c, buffer);
+	rc = get_user_bool(buffer, &bv);
 	if (rc)
 		return rc;
 
-	if (strtobool(&c, &bv) == 0) {
 #ifdef CONFIG_CIFS_STATS2
-		atomic_set(&totBufAllocCount, 0);
-		atomic_set(&totSmBufAllocCount, 0);
+	atomic_set(&totBufAllocCount, 0);
+	atomic_set(&totSmBufAllocCount, 0);
 #endif /* CONFIG_CIFS_STATS2 */
-		spin_lock(&cifs_tcp_ses_lock);
-		list_for_each(tmp1, &cifs_tcp_ses_list) {
-			server = list_entry(tmp1, struct TCP_Server_Info,
-					    tcp_ses_list);
-			list_for_each(tmp2, &server->smb_ses_list) {
-				ses = list_entry(tmp2, struct cifs_ses,
-						 smb_ses_list);
-				list_for_each(tmp3, &ses->tcon_list) {
-					tcon = list_entry(tmp3,
-							  struct cifs_tcon,
-							  tcon_list);
-					atomic_set(&tcon->num_smbs_sent, 0);
-					if (server->ops->clear_stats)
-						server->ops->clear_stats(tcon);
-				}
+	spin_lock(&cifs_tcp_ses_lock);
+	list_for_each(tmp1, &cifs_tcp_ses_list) {
+		server = list_entry(tmp1, struct TCP_Server_Info,
+				    tcp_ses_list);
+		list_for_each(tmp2, &server->smb_ses_list) {
+			ses = list_entry(tmp2, struct cifs_ses,
+					 smb_ses_list);
+			list_for_each(tmp3, &ses->tcon_list) {
+				tcon = list_entry(tmp3,
+						  struct cifs_tcon,
+						  tcon_list);
+				atomic_set(&tcon->num_smbs_sent, 0);
+				if (server->ops->clear_stats)
+					server->ops->clear_stats(tcon);
 			}
 		}
-		spin_unlock(&cifs_tcp_ses_lock);
 	}
+	spin_unlock(&cifs_tcp_ses_lock);
 
 	return count;
 }
@@ -433,17 +449,17 @@ static int cifsFYI_proc_open(struct inode *inode, struct file *file)
 static ssize_t cifsFYI_proc_write(struct file *file, const char __user *buffer,
 		size_t count, loff_t *ppos)
 {
-	char c;
+	char c[2] = { '\0' };
 	bool bv;
 	int rc;
 
-	rc = get_user(c, buffer);
+	rc = get_user(c[0], buffer);
 	if (rc)
 		return rc;
-	if (strtobool(&c, &bv) == 0)
+	if (strtobool(c, &bv) == 0)
 		cifsFYI = bv;
-	else if ((c > '1') && (c <= '9'))
-		cifsFYI = (int) (c - '0'); /* see cifs_debug.h for meanings */
+	else if ((c[0] > '1') && (c[0] <= '9'))
+		cifsFYI = (int) (c[0] - '0'); /* see cifs_debug.h for meanings */
 
 	return count;
 }
@@ -471,20 +487,12 @@ static int cifs_linux_ext_proc_open(struct inode *inode, struct file *file)
 static ssize_t cifs_linux_ext_proc_write(struct file *file,
 		const char __user *buffer, size_t count, loff_t *ppos)
 {
-	char c;
-	bool bv;
 	int rc;
 
-	rc = get_user(c, buffer);
+	rc = get_user_bool(buffer, &linuxExtEnabled);
 	if (rc)
 		return rc;
 
-	rc = strtobool(&c, &bv);
-	if (rc)
-		return rc;
-
-	linuxExtEnabled = bv;
-
 	return count;
 }
 
@@ -511,20 +519,12 @@ static int cifs_lookup_cache_proc_open(struct inode *inode, struct file *file)
 static ssize_t cifs_lookup_cache_proc_write(struct file *file,
 		const char __user *buffer, size_t count, loff_t *ppos)
 {
-	char c;
-	bool bv;
 	int rc;
 
-	rc = get_user(c, buffer);
+	rc = get_user_bool(buffer, &lookupCacheEnabled);
 	if (rc)
 		return rc;
 
-	rc = strtobool(&c, &bv);
-	if (rc)
-		return rc;
-
-	lookupCacheEnabled = bv;
-
 	return count;
 }
 
@@ -551,20 +551,12 @@ static int traceSMB_proc_open(struct inode *inode, struct file *file)
 static ssize_t traceSMB_proc_write(struct file *file, const char __user *buffer,
 		size_t count, loff_t *ppos)
 {
-	char c;
-	bool bv;
 	int rc;
 
-	rc = get_user(c, buffer);
+	rc = get_user_bool(buffer, &traceSMB);
 	if (rc)
 		return rc;
 
-	rc = strtobool(&c, &bv);
-	if (rc)
-		return rc;
-
-	traceSMB = bv;
-
 	return count;
 }
 
@@ -622,7 +614,7 @@ static ssize_t cifs_security_flags_proc_write(struct file *file,
 	int rc;
 	unsigned int flags;
 	char flags_string[12];
-	char c;
+	char c[2] = { '\0' };
 	bool bv;
 
 	if ((count < 1) || (count > 11))
@@ -635,11 +627,11 @@ static ssize_t cifs_security_flags_proc_write(struct file *file,
 
 	if (count < 3) {
 		/* single char or single char followed by null */
-		c = flags_string[0];
-		if (strtobool(&c, &bv) == 0) {
+		c[0] = flags_string[0];
+		if (strtobool(c, &bv) == 0) {
 			global_secflags = bv ? CIFSSEC_MAX : CIFSSEC_DEF;
 			return count;
-		} else if (!isdigit(c)) {
+		} else if (!isdigit(c[0])) {
 			cifs_dbg(VFS, "Invalid SecurityFlags: %s\n",
 					flags_string);
 			return -EINVAL;
diff --git a/fs/cifs/cifs_debug.h b/fs/cifs/cifs_debug.h
index 66cf0f9fff89..c611ca2339d7 100644
--- a/fs/cifs/cifs_debug.h
+++ b/fs/cifs/cifs_debug.h
@@ -25,7 +25,7 @@
 void cifs_dump_mem(char *label, void *data, int length);
 void cifs_dump_detail(void *);
 void cifs_dump_mids(struct TCP_Server_Info *);
-extern int traceSMB;		/* flag which enables the function below */
+extern bool traceSMB;		/* flag which enables the function below */
 void dump_smb(void *, int);
 #define CIFS_INFO	0x01
 #define CIFS_RC		0x02
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index c48ca13673e3..931b446f2a44 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -54,10 +54,10 @@
 #endif
 
 int cifsFYI = 0;
-int traceSMB = 0;
+bool traceSMB;
 bool enable_oplocks = true;
-unsigned int linuxExtEnabled = 1;
-unsigned int lookupCacheEnabled = 1;
+bool linuxExtEnabled = true;
+bool lookupCacheEnabled = true;
 unsigned int global_secflags = CIFSSEC_DEF;
 /* unsigned int ntlmv2_support = 0; */
 unsigned int sign_CIFS_PDUs = 1;
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index a25b2513f146..d21da9f05bae 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1596,11 +1596,11 @@ GLOBAL_EXTERN atomic_t midCount;
 
 /* Misc globals */
 GLOBAL_EXTERN bool enable_oplocks; /* enable or disable oplocks */
-GLOBAL_EXTERN unsigned int lookupCacheEnabled;
+GLOBAL_EXTERN bool lookupCacheEnabled;
 GLOBAL_EXTERN unsigned int global_secflags;	/* if on, session setup sent
 				with more secure ntlmssp2 challenge/resp */
 GLOBAL_EXTERN unsigned int sign_CIFS_PDUs;  /* enable smb packet signing */
-GLOBAL_EXTERN unsigned int linuxExtEnabled;/*enable Linux/Unix CIFS extensions*/
+GLOBAL_EXTERN bool linuxExtEnabled;/*enable Linux/Unix CIFS extensions*/
 GLOBAL_EXTERN unsigned int CIFSMaxBufSize;  /* max size not including hdr */
 GLOBAL_EXTERN unsigned int cifs_min_rcv;    /* min size of big ntwrk buf pool */
 GLOBAL_EXTERN unsigned int cifs_min_small;  /* min size of small buf pool */
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 1/3] lib: fix callers of strtobool to use char array
@ 2016-01-28 14:17   ` Kees Cook
  0 siblings, 0 replies; 14+ messages in thread
From: Kees Cook @ 2016-01-28 14:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Amitkumar Karwar, Nishant Sarmukadam, Kalle Valo,
	Steve French, linux-cifs-u79uwXL29TY76Z2rM5mHXA, Joe Perches,
	Rasmus Villemoes, Daniel Borkmann, Michael Ellerman,
	Heiko Carstens, Martin Schwidefsky, x86-DgEjT+Ai2ygdnm+yROfE0A,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-s390-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Some callers of strtobool were passing a pointer to unterminated strings.
This fixes the issue and consolidates some logic in cifs.

Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Amitkumar Karwar <akarwar-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
Cc: Nishant Sarmukadam <nishants-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
Cc: Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Cc: Steve French <sfrench-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
 drivers/net/wireless/marvell/mwifiex/debugfs.c |   6 +-
 fs/cifs/cifs_debug.c                           | 106 ++++++++++++-------------
 fs/cifs/cifs_debug.h                           |   2 +-
 fs/cifs/cifsfs.c                               |   6 +-
 fs/cifs/cifsglob.h                             |   4 +-
 5 files changed, 58 insertions(+), 66 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/debugfs.c b/drivers/net/wireless/marvell/mwifiex/debugfs.c
index 0b9c580af988..76af60899c69 100644
--- a/drivers/net/wireless/marvell/mwifiex/debugfs.c
+++ b/drivers/net/wireless/marvell/mwifiex/debugfs.c
@@ -880,13 +880,13 @@ mwifiex_reset_write(struct file *file,
 {
 	struct mwifiex_private *priv = file->private_data;
 	struct mwifiex_adapter *adapter = priv->adapter;
-	char cmd;
+	char cmd[2] = { '\0' };
 	bool result;
 
-	if (copy_from_user(&cmd, ubuf, sizeof(cmd)))
+	if (copy_from_user(cmd, ubuf, sizeof(char)))
 		return -EFAULT;
 
-	if (strtobool(&cmd, &result))
+	if (strtobool(cmd, &result))
 		return -EINVAL;
 
 	if (!result)
diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
index 50b268483302..2f7ffcc9e364 100644
--- a/fs/cifs/cifs_debug.c
+++ b/fs/cifs/cifs_debug.c
@@ -251,11 +251,29 @@ static const struct file_operations cifs_debug_data_proc_fops = {
 	.release	= single_release,
 };
 
+static int get_user_bool(const char __user *buffer, bool *store)
+{
+	char c[2] = { '\0' };
+	bool bv;
+	int rc;
+
+	rc = get_user(c[0], buffer);
+	if (rc)
+		return rc;
+
+	rc = strtobool(c, &bv);
+	if (rc)
+		return rc;
+
+	*store = bv;
+
+	return 0;
+}
+
 #ifdef CONFIG_CIFS_STATS
 static ssize_t cifs_stats_proc_write(struct file *file,
 		const char __user *buffer, size_t count, loff_t *ppos)
 {
-	char c;
 	bool bv;
 	int rc;
 	struct list_head *tmp1, *tmp2, *tmp3;
@@ -263,34 +281,32 @@ static ssize_t cifs_stats_proc_write(struct file *file,
 	struct cifs_ses *ses;
 	struct cifs_tcon *tcon;
 
-	rc = get_user(c, buffer);
+	rc = get_user_bool(buffer, &bv);
 	if (rc)
 		return rc;
 
-	if (strtobool(&c, &bv) == 0) {
 #ifdef CONFIG_CIFS_STATS2
-		atomic_set(&totBufAllocCount, 0);
-		atomic_set(&totSmBufAllocCount, 0);
+	atomic_set(&totBufAllocCount, 0);
+	atomic_set(&totSmBufAllocCount, 0);
 #endif /* CONFIG_CIFS_STATS2 */
-		spin_lock(&cifs_tcp_ses_lock);
-		list_for_each(tmp1, &cifs_tcp_ses_list) {
-			server = list_entry(tmp1, struct TCP_Server_Info,
-					    tcp_ses_list);
-			list_for_each(tmp2, &server->smb_ses_list) {
-				ses = list_entry(tmp2, struct cifs_ses,
-						 smb_ses_list);
-				list_for_each(tmp3, &ses->tcon_list) {
-					tcon = list_entry(tmp3,
-							  struct cifs_tcon,
-							  tcon_list);
-					atomic_set(&tcon->num_smbs_sent, 0);
-					if (server->ops->clear_stats)
-						server->ops->clear_stats(tcon);
-				}
+	spin_lock(&cifs_tcp_ses_lock);
+	list_for_each(tmp1, &cifs_tcp_ses_list) {
+		server = list_entry(tmp1, struct TCP_Server_Info,
+				    tcp_ses_list);
+		list_for_each(tmp2, &server->smb_ses_list) {
+			ses = list_entry(tmp2, struct cifs_ses,
+					 smb_ses_list);
+			list_for_each(tmp3, &ses->tcon_list) {
+				tcon = list_entry(tmp3,
+						  struct cifs_tcon,
+						  tcon_list);
+				atomic_set(&tcon->num_smbs_sent, 0);
+				if (server->ops->clear_stats)
+					server->ops->clear_stats(tcon);
 			}
 		}
-		spin_unlock(&cifs_tcp_ses_lock);
 	}
+	spin_unlock(&cifs_tcp_ses_lock);
 
 	return count;
 }
@@ -433,17 +449,17 @@ static int cifsFYI_proc_open(struct inode *inode, struct file *file)
 static ssize_t cifsFYI_proc_write(struct file *file, const char __user *buffer,
 		size_t count, loff_t *ppos)
 {
-	char c;
+	char c[2] = { '\0' };
 	bool bv;
 	int rc;
 
-	rc = get_user(c, buffer);
+	rc = get_user(c[0], buffer);
 	if (rc)
 		return rc;
-	if (strtobool(&c, &bv) == 0)
+	if (strtobool(c, &bv) == 0)
 		cifsFYI = bv;
-	else if ((c > '1') && (c <= '9'))
-		cifsFYI = (int) (c - '0'); /* see cifs_debug.h for meanings */
+	else if ((c[0] > '1') && (c[0] <= '9'))
+		cifsFYI = (int) (c[0] - '0'); /* see cifs_debug.h for meanings */
 
 	return count;
 }
@@ -471,20 +487,12 @@ static int cifs_linux_ext_proc_open(struct inode *inode, struct file *file)
 static ssize_t cifs_linux_ext_proc_write(struct file *file,
 		const char __user *buffer, size_t count, loff_t *ppos)
 {
-	char c;
-	bool bv;
 	int rc;
 
-	rc = get_user(c, buffer);
+	rc = get_user_bool(buffer, &linuxExtEnabled);
 	if (rc)
 		return rc;
 
-	rc = strtobool(&c, &bv);
-	if (rc)
-		return rc;
-
-	linuxExtEnabled = bv;
-
 	return count;
 }
 
@@ -511,20 +519,12 @@ static int cifs_lookup_cache_proc_open(struct inode *inode, struct file *file)
 static ssize_t cifs_lookup_cache_proc_write(struct file *file,
 		const char __user *buffer, size_t count, loff_t *ppos)
 {
-	char c;
-	bool bv;
 	int rc;
 
-	rc = get_user(c, buffer);
+	rc = get_user_bool(buffer, &lookupCacheEnabled);
 	if (rc)
 		return rc;
 
-	rc = strtobool(&c, &bv);
-	if (rc)
-		return rc;
-
-	lookupCacheEnabled = bv;
-
 	return count;
 }
 
@@ -551,20 +551,12 @@ static int traceSMB_proc_open(struct inode *inode, struct file *file)
 static ssize_t traceSMB_proc_write(struct file *file, const char __user *buffer,
 		size_t count, loff_t *ppos)
 {
-	char c;
-	bool bv;
 	int rc;
 
-	rc = get_user(c, buffer);
+	rc = get_user_bool(buffer, &traceSMB);
 	if (rc)
 		return rc;
 
-	rc = strtobool(&c, &bv);
-	if (rc)
-		return rc;
-
-	traceSMB = bv;
-
 	return count;
 }
 
@@ -622,7 +614,7 @@ static ssize_t cifs_security_flags_proc_write(struct file *file,
 	int rc;
 	unsigned int flags;
 	char flags_string[12];
-	char c;
+	char c[2] = { '\0' };
 	bool bv;
 
 	if ((count < 1) || (count > 11))
@@ -635,11 +627,11 @@ static ssize_t cifs_security_flags_proc_write(struct file *file,
 
 	if (count < 3) {
 		/* single char or single char followed by null */
-		c = flags_string[0];
-		if (strtobool(&c, &bv) == 0) {
+		c[0] = flags_string[0];
+		if (strtobool(c, &bv) == 0) {
 			global_secflags = bv ? CIFSSEC_MAX : CIFSSEC_DEF;
 			return count;
-		} else if (!isdigit(c)) {
+		} else if (!isdigit(c[0])) {
 			cifs_dbg(VFS, "Invalid SecurityFlags: %s\n",
 					flags_string);
 			return -EINVAL;
diff --git a/fs/cifs/cifs_debug.h b/fs/cifs/cifs_debug.h
index 66cf0f9fff89..c611ca2339d7 100644
--- a/fs/cifs/cifs_debug.h
+++ b/fs/cifs/cifs_debug.h
@@ -25,7 +25,7 @@
 void cifs_dump_mem(char *label, void *data, int length);
 void cifs_dump_detail(void *);
 void cifs_dump_mids(struct TCP_Server_Info *);
-extern int traceSMB;		/* flag which enables the function below */
+extern bool traceSMB;		/* flag which enables the function below */
 void dump_smb(void *, int);
 #define CIFS_INFO	0x01
 #define CIFS_RC		0x02
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index c48ca13673e3..931b446f2a44 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -54,10 +54,10 @@
 #endif
 
 int cifsFYI = 0;
-int traceSMB = 0;
+bool traceSMB;
 bool enable_oplocks = true;
-unsigned int linuxExtEnabled = 1;
-unsigned int lookupCacheEnabled = 1;
+bool linuxExtEnabled = true;
+bool lookupCacheEnabled = true;
 unsigned int global_secflags = CIFSSEC_DEF;
 /* unsigned int ntlmv2_support = 0; */
 unsigned int sign_CIFS_PDUs = 1;
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index a25b2513f146..d21da9f05bae 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1596,11 +1596,11 @@ GLOBAL_EXTERN atomic_t midCount;
 
 /* Misc globals */
 GLOBAL_EXTERN bool enable_oplocks; /* enable or disable oplocks */
-GLOBAL_EXTERN unsigned int lookupCacheEnabled;
+GLOBAL_EXTERN bool lookupCacheEnabled;
 GLOBAL_EXTERN unsigned int global_secflags;	/* if on, session setup sent
 				with more secure ntlmssp2 challenge/resp */
 GLOBAL_EXTERN unsigned int sign_CIFS_PDUs;  /* enable smb packet signing */
-GLOBAL_EXTERN unsigned int linuxExtEnabled;/*enable Linux/Unix CIFS extensions*/
+GLOBAL_EXTERN bool linuxExtEnabled;/*enable Linux/Unix CIFS extensions*/
 GLOBAL_EXTERN unsigned int CIFSMaxBufSize;  /* max size not including hdr */
 GLOBAL_EXTERN unsigned int cifs_min_rcv;    /* min size of big ntwrk buf pool */
 GLOBAL_EXTERN unsigned int cifs_min_small;  /* min size of small buf pool */
-- 
2.6.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 2/3] lib: add "on" and "off" to strtobool
  2016-01-28 14:17 ` Kees Cook
  (?)
  (?)
@ 2016-01-28 14:17 ` Kees Cook
  -1 siblings, 0 replies; 14+ messages in thread
From: Kees Cook @ 2016-01-28 14:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Rasmus Villemoes, Daniel Borkmann, Joe Perches,
	Amitkumar Karwar, Nishant Sarmukadam, Kalle Valo, Steve French,
	Michael Ellerman, Heiko Carstens, Martin Schwidefsky, x86,
	linuxppc-dev, linux-s390, linux-wireless, netdev, linux-cifs,
	linux-kernel

Several places in the kernel expect to use "on" and "off" for their
boolean signifiers, so add them to strtobool.

Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Daniel Borkmann <daniel@iogearbox.net>
---
 lib/string.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/lib/string.c b/lib/string.c
index 0323c0d5629a..091570708db7 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -635,12 +635,15 @@ EXPORT_SYMBOL(sysfs_streq);
  * @s: input string
  * @res: result
  *
- * This routine returns 0 iff the first character is one of 'Yy1Nn0'.
- * Otherwise it will return -EINVAL.  Value pointed to by res is
- * updated upon finding a match.
+ * This routine returns 0 iff the first character is one of 'Yy1Nn0', or
+ * [oO][NnFf] for "on" and "off". Otherwise it will return -EINVAL.  Value
+ * pointed to by res is updated upon finding a match.
  */
 int strtobool(const char *s, bool *res)
 {
+	if (!s)
+		return -EINVAL;
+
 	switch (s[0]) {
 	case 'y':
 	case 'Y':
@@ -652,6 +655,21 @@ int strtobool(const char *s, bool *res)
 	case '0':
 		*res = false;
 		break;
+	case 'o':
+	case 'O':
+		switch (s[1]) {
+		case 'n':
+		case 'N':
+			*res = true;
+			break;
+		case 'f':
+		case 'F':
+			*res = false;
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
 	default:
 		return -EINVAL;
 	}
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 3/3] param: convert some "on"/"off" users to strtobool
@ 2016-01-28 14:17   ` Kees Cook
  0 siblings, 0 replies; 14+ messages in thread
From: Kees Cook @ 2016-01-28 14:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, x86, linuxppc-dev, linux-s390, Joe Perches,
	Rasmus Villemoes, Daniel Borkmann, Amitkumar Karwar,
	Nishant Sarmukadam, Kalle Valo, Steve French, Michael Ellerman,
	Heiko Carstens, Martin Schwidefsky, linux-wireless, netdev,
	linux-cifs, linux-kernel

This changes several users of manual "on"/"off" parsing to use strtobool.

Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: x86@kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-s390@vger.kernel.org
---
 arch/powerpc/kernel/rtasd.c                  | 10 +++-------
 arch/powerpc/platforms/pseries/hotplug-cpu.c | 11 +++--------
 arch/s390/kernel/time.c                      |  8 ++------
 arch/s390/kernel/topology.c                  |  8 +++-----
 arch/x86/kernel/aperture_64.c                | 13 +++----------
 include/linux/tick.h                         |  2 +-
 kernel/time/hrtimer.c                        | 11 +++--------
 kernel/time/tick-sched.c                     | 11 +++--------
 8 files changed, 21 insertions(+), 53 deletions(-)

diff --git a/arch/powerpc/kernel/rtasd.c b/arch/powerpc/kernel/rtasd.c
index 5a2c049c1c61..984e67e91ba3 100644
--- a/arch/powerpc/kernel/rtasd.c
+++ b/arch/powerpc/kernel/rtasd.c
@@ -21,6 +21,7 @@
 #include <linux/cpu.h>
 #include <linux/workqueue.h>
 #include <linux/slab.h>
+#include <linux/string.h>
 
 #include <asm/uaccess.h>
 #include <asm/io.h>
@@ -49,7 +50,7 @@ static unsigned int rtas_error_log_buffer_max;
 static unsigned int event_scan;
 static unsigned int rtas_event_scan_rate;
 
-static int full_rtas_msgs = 0;
+static bool full_rtas_msgs;
 
 /* Stop logging to nvram after first fatal error */
 static int logging_enabled; /* Until we initialize everything,
@@ -592,11 +593,6 @@ __setup("surveillance=", surveillance_setup);
 
 static int __init rtasmsgs_setup(char *str)
 {
-	if (strcmp(str, "on") == 0)
-		full_rtas_msgs = 1;
-	else if (strcmp(str, "off") == 0)
-		full_rtas_msgs = 0;
-
-	return 1;
+	return strtobool(str, &full_rtas_msgs);
 }
 __setup("rtasmsgs=", rtasmsgs_setup);
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 32274f72fe3f..bb333e9fd77a 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -27,6 +27,7 @@
 #include <linux/cpu.h>
 #include <linux/of.h>
 #include <linux/slab.h>
+#include <linux/string.h>
 #include <asm/prom.h>
 #include <asm/rtas.h>
 #include <asm/firmware.h>
@@ -47,20 +48,14 @@ static DEFINE_PER_CPU(enum cpu_state_vals, current_state) = CPU_STATE_OFFLINE;
 
 static enum cpu_state_vals default_offline_state = CPU_STATE_OFFLINE;
 
-static int cede_offline_enabled __read_mostly = 1;
+static bool cede_offline_enabled __read_mostly = true;
 
 /*
  * Enable/disable cede_offline when available.
  */
 static int __init setup_cede_offline(char *str)
 {
-	if (!strcmp(str, "off"))
-		cede_offline_enabled = 0;
-	else if (!strcmp(str, "on"))
-		cede_offline_enabled = 1;
-	else
-		return 0;
-	return 1;
+	return strtobool(str, &cede_offline_enabled);
 }
 
 __setup("cede_offline=", setup_cede_offline);
diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c
index 99f84ac31307..afc7fc9684ba 100644
--- a/arch/s390/kernel/time.c
+++ b/arch/s390/kernel/time.c
@@ -1433,7 +1433,7 @@ device_initcall(etr_init_sysfs);
 /*
  * Server Time Protocol (STP) code.
  */
-static int stp_online;
+static bool stp_online;
 static struct stp_sstpi stp_info;
 static void *stp_page;
 
@@ -1444,11 +1444,7 @@ static struct timer_list stp_timer;
 
 static int __init early_parse_stp(char *p)
 {
-	if (strncmp(p, "off", 3) == 0)
-		stp_online = 0;
-	else if (strncmp(p, "on", 2) == 0)
-		stp_online = 1;
-	return 0;
+	return strtobool(p, &stp_online);
 }
 early_param("stp", early_parse_stp);
 
diff --git a/arch/s390/kernel/topology.c b/arch/s390/kernel/topology.c
index 40b8102fdadb..10e388216307 100644
--- a/arch/s390/kernel/topology.c
+++ b/arch/s390/kernel/topology.c
@@ -15,6 +15,7 @@
 #include <linux/delay.h>
 #include <linux/init.h>
 #include <linux/slab.h>
+#include <linux/string.h>
 #include <linux/cpu.h>
 #include <linux/smp.h>
 #include <linux/mm.h>
@@ -37,7 +38,7 @@ static void set_topology_timer(void);
 static void topology_work_fn(struct work_struct *work);
 static struct sysinfo_15_1_x *tl_info;
 
-static int topology_enabled = 1;
+static bool topology_enabled = true;
 static DECLARE_WORK(topology_work, topology_work_fn);
 
 /*
@@ -444,10 +445,7 @@ static const struct cpumask *cpu_book_mask(int cpu)
 
 static int __init early_parse_topology(char *p)
 {
-	if (strncmp(p, "off", 3))
-		return 0;
-	topology_enabled = 0;
-	return 0;
+	return strtobool(p, &topology_enabled);
 }
 early_param("topology", early_parse_topology);
 
diff --git a/arch/x86/kernel/aperture_64.c b/arch/x86/kernel/aperture_64.c
index 6e85f713641d..6608b00a516a 100644
--- a/arch/x86/kernel/aperture_64.c
+++ b/arch/x86/kernel/aperture_64.c
@@ -20,6 +20,7 @@
 #include <linux/pci_ids.h>
 #include <linux/pci.h>
 #include <linux/bitops.h>
+#include <linux/string.h>
 #include <linux/suspend.h>
 #include <asm/e820.h>
 #include <asm/io.h>
@@ -227,19 +228,11 @@ static u32 __init search_agp_bridge(u32 *order, int *valid_agp)
 	return 0;
 }
 
-static int gart_fix_e820 __initdata = 1;
+static bool gart_fix_e820 __initdata = true;
 
 static int __init parse_gart_mem(char *p)
 {
-	if (!p)
-		return -EINVAL;
-
-	if (!strncmp(p, "off", 3))
-		gart_fix_e820 = 0;
-	else if (!strncmp(p, "on", 2))
-		gart_fix_e820 = 1;
-
-	return 0;
+	return strtobool(p, &gart_fix_e820);
 }
 early_param("gart_fix_e820", parse_gart_mem);
 
diff --git a/include/linux/tick.h b/include/linux/tick.h
index 97fd4e543846..0ecdf0e248f4 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -98,7 +98,7 @@ static inline void tick_broadcast_exit(void)
 }
 
 #ifdef CONFIG_NO_HZ_COMMON
-extern int tick_nohz_enabled;
+extern bool tick_nohz_enabled;
 extern int tick_nohz_tick_stopped(void);
 extern void tick_nohz_idle_enter(void);
 extern void tick_nohz_idle_exit(void);
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 435b8850dd80..40d82fe4d2a5 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -39,6 +39,7 @@
 #include <linux/syscalls.h>
 #include <linux/kallsyms.h>
 #include <linux/interrupt.h>
+#include <linux/string.h>
 #include <linux/tick.h>
 #include <linux/seq_file.h>
 #include <linux/err.h>
@@ -515,7 +516,7 @@ static inline ktime_t hrtimer_update_base(struct hrtimer_cpu_base *base)
 /*
  * High resolution timer enabled ?
  */
-static int hrtimer_hres_enabled __read_mostly  = 1;
+static bool hrtimer_hres_enabled __read_mostly  = true;
 unsigned int hrtimer_resolution __read_mostly = LOW_RES_NSEC;
 EXPORT_SYMBOL_GPL(hrtimer_resolution);
 
@@ -524,13 +525,7 @@ EXPORT_SYMBOL_GPL(hrtimer_resolution);
  */
 static int __init setup_hrtimer_hres(char *str)
 {
-	if (!strcmp(str, "off"))
-		hrtimer_hres_enabled = 0;
-	else if (!strcmp(str, "on"))
-		hrtimer_hres_enabled = 1;
-	else
-		return 0;
-	return 1;
+	return strtobool(str, &hrtimer_hres_enabled);
 }
 
 __setup("highres=", setup_hrtimer_hres);
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 9d7a053545f5..bd97702a0760 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -19,6 +19,7 @@
 #include <linux/percpu.h>
 #include <linux/profile.h>
 #include <linux/sched.h>
+#include <linux/string.h>
 #include <linux/module.h>
 #include <linux/irq_work.h>
 #include <linux/posix-timers.h>
@@ -387,20 +388,14 @@ void __init tick_nohz_init(void)
 /*
  * NO HZ enabled ?
  */
-int tick_nohz_enabled __read_mostly = 1;
+bool tick_nohz_enabled __read_mostly  = true;
 unsigned long tick_nohz_active  __read_mostly;
 /*
  * Enable / Disable tickless mode
  */
 static int __init setup_tick_nohz(char *str)
 {
-	if (!strcmp(str, "off"))
-		tick_nohz_enabled = 0;
-	else if (!strcmp(str, "on"))
-		tick_nohz_enabled = 1;
-	else
-		return 0;
-	return 1;
+	return strtobool(str, &tick_nohz_enabled);
 }
 
 __setup("nohz=", setup_tick_nohz);
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 3/3] param: convert some "on"/"off" users to strtobool
@ 2016-01-28 14:17   ` Kees Cook
  0 siblings, 0 replies; 14+ messages in thread
From: Kees Cook @ 2016-01-28 14:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, x86-DgEjT+Ai2ygdnm+yROfE0A,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-s390-u79uwXL29TY76Z2rM5mHXA, Joe Perches, Rasmus Villemoes,
	Daniel Borkmann, Amitkumar Karwar, Nishant Sarmukadam,
	Kalle Valo, Steve French, Michael Ellerman, Heiko Carstens,
	Martin Schwidefsky, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

This changes several users of manual "on"/"off" parsing to use strtobool.

Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Cc: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
Cc: linux-s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
 arch/powerpc/kernel/rtasd.c                  | 10 +++-------
 arch/powerpc/platforms/pseries/hotplug-cpu.c | 11 +++--------
 arch/s390/kernel/time.c                      |  8 ++------
 arch/s390/kernel/topology.c                  |  8 +++-----
 arch/x86/kernel/aperture_64.c                | 13 +++----------
 include/linux/tick.h                         |  2 +-
 kernel/time/hrtimer.c                        | 11 +++--------
 kernel/time/tick-sched.c                     | 11 +++--------
 8 files changed, 21 insertions(+), 53 deletions(-)

diff --git a/arch/powerpc/kernel/rtasd.c b/arch/powerpc/kernel/rtasd.c
index 5a2c049c1c61..984e67e91ba3 100644
--- a/arch/powerpc/kernel/rtasd.c
+++ b/arch/powerpc/kernel/rtasd.c
@@ -21,6 +21,7 @@
 #include <linux/cpu.h>
 #include <linux/workqueue.h>
 #include <linux/slab.h>
+#include <linux/string.h>
 
 #include <asm/uaccess.h>
 #include <asm/io.h>
@@ -49,7 +50,7 @@ static unsigned int rtas_error_log_buffer_max;
 static unsigned int event_scan;
 static unsigned int rtas_event_scan_rate;
 
-static int full_rtas_msgs = 0;
+static bool full_rtas_msgs;
 
 /* Stop logging to nvram after first fatal error */
 static int logging_enabled; /* Until we initialize everything,
@@ -592,11 +593,6 @@ __setup("surveillance=", surveillance_setup);
 
 static int __init rtasmsgs_setup(char *str)
 {
-	if (strcmp(str, "on") == 0)
-		full_rtas_msgs = 1;
-	else if (strcmp(str, "off") == 0)
-		full_rtas_msgs = 0;
-
-	return 1;
+	return strtobool(str, &full_rtas_msgs);
 }
 __setup("rtasmsgs=", rtasmsgs_setup);
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 32274f72fe3f..bb333e9fd77a 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -27,6 +27,7 @@
 #include <linux/cpu.h>
 #include <linux/of.h>
 #include <linux/slab.h>
+#include <linux/string.h>
 #include <asm/prom.h>
 #include <asm/rtas.h>
 #include <asm/firmware.h>
@@ -47,20 +48,14 @@ static DEFINE_PER_CPU(enum cpu_state_vals, current_state) = CPU_STATE_OFFLINE;
 
 static enum cpu_state_vals default_offline_state = CPU_STATE_OFFLINE;
 
-static int cede_offline_enabled __read_mostly = 1;
+static bool cede_offline_enabled __read_mostly = true;
 
 /*
  * Enable/disable cede_offline when available.
  */
 static int __init setup_cede_offline(char *str)
 {
-	if (!strcmp(str, "off"))
-		cede_offline_enabled = 0;
-	else if (!strcmp(str, "on"))
-		cede_offline_enabled = 1;
-	else
-		return 0;
-	return 1;
+	return strtobool(str, &cede_offline_enabled);
 }
 
 __setup("cede_offline=", setup_cede_offline);
diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c
index 99f84ac31307..afc7fc9684ba 100644
--- a/arch/s390/kernel/time.c
+++ b/arch/s390/kernel/time.c
@@ -1433,7 +1433,7 @@ device_initcall(etr_init_sysfs);
 /*
  * Server Time Protocol (STP) code.
  */
-static int stp_online;
+static bool stp_online;
 static struct stp_sstpi stp_info;
 static void *stp_page;
 
@@ -1444,11 +1444,7 @@ static struct timer_list stp_timer;
 
 static int __init early_parse_stp(char *p)
 {
-	if (strncmp(p, "off", 3) == 0)
-		stp_online = 0;
-	else if (strncmp(p, "on", 2) == 0)
-		stp_online = 1;
-	return 0;
+	return strtobool(p, &stp_online);
 }
 early_param("stp", early_parse_stp);
 
diff --git a/arch/s390/kernel/topology.c b/arch/s390/kernel/topology.c
index 40b8102fdadb..10e388216307 100644
--- a/arch/s390/kernel/topology.c
+++ b/arch/s390/kernel/topology.c
@@ -15,6 +15,7 @@
 #include <linux/delay.h>
 #include <linux/init.h>
 #include <linux/slab.h>
+#include <linux/string.h>
 #include <linux/cpu.h>
 #include <linux/smp.h>
 #include <linux/mm.h>
@@ -37,7 +38,7 @@ static void set_topology_timer(void);
 static void topology_work_fn(struct work_struct *work);
 static struct sysinfo_15_1_x *tl_info;
 
-static int topology_enabled = 1;
+static bool topology_enabled = true;
 static DECLARE_WORK(topology_work, topology_work_fn);
 
 /*
@@ -444,10 +445,7 @@ static const struct cpumask *cpu_book_mask(int cpu)
 
 static int __init early_parse_topology(char *p)
 {
-	if (strncmp(p, "off", 3))
-		return 0;
-	topology_enabled = 0;
-	return 0;
+	return strtobool(p, &topology_enabled);
 }
 early_param("topology", early_parse_topology);
 
diff --git a/arch/x86/kernel/aperture_64.c b/arch/x86/kernel/aperture_64.c
index 6e85f713641d..6608b00a516a 100644
--- a/arch/x86/kernel/aperture_64.c
+++ b/arch/x86/kernel/aperture_64.c
@@ -20,6 +20,7 @@
 #include <linux/pci_ids.h>
 #include <linux/pci.h>
 #include <linux/bitops.h>
+#include <linux/string.h>
 #include <linux/suspend.h>
 #include <asm/e820.h>
 #include <asm/io.h>
@@ -227,19 +228,11 @@ static u32 __init search_agp_bridge(u32 *order, int *valid_agp)
 	return 0;
 }
 
-static int gart_fix_e820 __initdata = 1;
+static bool gart_fix_e820 __initdata = true;
 
 static int __init parse_gart_mem(char *p)
 {
-	if (!p)
-		return -EINVAL;
-
-	if (!strncmp(p, "off", 3))
-		gart_fix_e820 = 0;
-	else if (!strncmp(p, "on", 2))
-		gart_fix_e820 = 1;
-
-	return 0;
+	return strtobool(p, &gart_fix_e820);
 }
 early_param("gart_fix_e820", parse_gart_mem);
 
diff --git a/include/linux/tick.h b/include/linux/tick.h
index 97fd4e543846..0ecdf0e248f4 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -98,7 +98,7 @@ static inline void tick_broadcast_exit(void)
 }
 
 #ifdef CONFIG_NO_HZ_COMMON
-extern int tick_nohz_enabled;
+extern bool tick_nohz_enabled;
 extern int tick_nohz_tick_stopped(void);
 extern void tick_nohz_idle_enter(void);
 extern void tick_nohz_idle_exit(void);
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 435b8850dd80..40d82fe4d2a5 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -39,6 +39,7 @@
 #include <linux/syscalls.h>
 #include <linux/kallsyms.h>
 #include <linux/interrupt.h>
+#include <linux/string.h>
 #include <linux/tick.h>
 #include <linux/seq_file.h>
 #include <linux/err.h>
@@ -515,7 +516,7 @@ static inline ktime_t hrtimer_update_base(struct hrtimer_cpu_base *base)
 /*
  * High resolution timer enabled ?
  */
-static int hrtimer_hres_enabled __read_mostly  = 1;
+static bool hrtimer_hres_enabled __read_mostly  = true;
 unsigned int hrtimer_resolution __read_mostly = LOW_RES_NSEC;
 EXPORT_SYMBOL_GPL(hrtimer_resolution);
 
@@ -524,13 +525,7 @@ EXPORT_SYMBOL_GPL(hrtimer_resolution);
  */
 static int __init setup_hrtimer_hres(char *str)
 {
-	if (!strcmp(str, "off"))
-		hrtimer_hres_enabled = 0;
-	else if (!strcmp(str, "on"))
-		hrtimer_hres_enabled = 1;
-	else
-		return 0;
-	return 1;
+	return strtobool(str, &hrtimer_hres_enabled);
 }
 
 __setup("highres=", setup_hrtimer_hres);
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 9d7a053545f5..bd97702a0760 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -19,6 +19,7 @@
 #include <linux/percpu.h>
 #include <linux/profile.h>
 #include <linux/sched.h>
+#include <linux/string.h>
 #include <linux/module.h>
 #include <linux/irq_work.h>
 #include <linux/posix-timers.h>
@@ -387,20 +388,14 @@ void __init tick_nohz_init(void)
 /*
  * NO HZ enabled ?
  */
-int tick_nohz_enabled __read_mostly = 1;
+bool tick_nohz_enabled __read_mostly  = true;
 unsigned long tick_nohz_active  __read_mostly;
 /*
  * Enable / Disable tickless mode
  */
 static int __init setup_tick_nohz(char *str)
 {
-	if (!strcmp(str, "off"))
-		tick_nohz_enabled = 0;
-	else if (!strcmp(str, "on"))
-		tick_nohz_enabled = 1;
-	else
-		return 0;
-	return 1;
+	return strtobool(str, &tick_nohz_enabled);
 }
 
 __setup("nohz=", setup_tick_nohz);
-- 
2.6.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] param: convert some "on"/"off" users to strtobool
@ 2016-01-28 15:27     ` Heiko Carstens
  0 siblings, 0 replies; 14+ messages in thread
From: Heiko Carstens @ 2016-01-28 15:27 UTC (permalink / raw)
  To: Kees Cook
  Cc: Andrew Morton, x86, linuxppc-dev, linux-s390, Joe Perches,
	Rasmus Villemoes, Daniel Borkmann, Amitkumar Karwar,
	Nishant Sarmukadam, Kalle Valo, Steve French, Michael Ellerman,
	Martin Schwidefsky, linux-wireless, netdev, linux-cifs,
	linux-kernel

On Thu, Jan 28, 2016 at 06:17:07AM -0800, Kees Cook wrote:
> This changes several users of manual "on"/"off" parsing to use strtobool.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Cc: x86@kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-s390@vger.kernel.org
> ---
>  arch/powerpc/kernel/rtasd.c                  | 10 +++-------
>  arch/powerpc/platforms/pseries/hotplug-cpu.c | 11 +++--------
>  arch/s390/kernel/time.c                      |  8 ++------
>  arch/s390/kernel/topology.c                  |  8 +++-----
>  arch/x86/kernel/aperture_64.c                | 13 +++----------
>  include/linux/tick.h                         |  2 +-
>  kernel/time/hrtimer.c                        | 11 +++--------
>  kernel/time/tick-sched.c                     | 11 +++--------
>  8 files changed, 21 insertions(+), 53 deletions(-)

For the s390 bits:

Acked-by: Heiko Carstens <heiko.carstens@de.ibm.com>


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] param: convert some "on"/"off" users to strtobool
@ 2016-01-28 15:27     ` Heiko Carstens
  0 siblings, 0 replies; 14+ messages in thread
From: Heiko Carstens @ 2016-01-28 15:27 UTC (permalink / raw)
  To: Kees Cook
  Cc: Andrew Morton, x86-DgEjT+Ai2ygdnm+yROfE0A,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-s390-u79uwXL29TY76Z2rM5mHXA, Joe Perches, Rasmus Villemoes,
	Daniel Borkmann, Amitkumar Karwar, Nishant Sarmukadam,
	Kalle Valo, Steve French, Michael Ellerman, Martin Schwidefsky,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Thu, Jan 28, 2016 at 06:17:07AM -0800, Kees Cook wrote:
> This changes several users of manual "on"/"off" parsing to use strtobool.
> 
> Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Cc: x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
> Cc: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> Cc: linux-s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  arch/powerpc/kernel/rtasd.c                  | 10 +++-------
>  arch/powerpc/platforms/pseries/hotplug-cpu.c | 11 +++--------
>  arch/s390/kernel/time.c                      |  8 ++------
>  arch/s390/kernel/topology.c                  |  8 +++-----
>  arch/x86/kernel/aperture_64.c                | 13 +++----------
>  include/linux/tick.h                         |  2 +-
>  kernel/time/hrtimer.c                        | 11 +++--------
>  kernel/time/tick-sched.c                     | 11 +++--------
>  8 files changed, 21 insertions(+), 53 deletions(-)

For the s390 bits:

Acked-by: Heiko Carstens <heiko.carstens-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] param: convert some "on"/"off" users to strtobool
@ 2016-01-29  3:15     ` Michael Ellerman
  0 siblings, 0 replies; 14+ messages in thread
From: Michael Ellerman @ 2016-01-29  3:15 UTC (permalink / raw)
  To: Kees Cook, Andrew Morton
  Cc: x86, linuxppc-dev, linux-s390, Joe Perches, Rasmus Villemoes,
	Daniel Borkmann, Amitkumar Karwar, Nishant Sarmukadam,
	Kalle Valo, Steve French, Heiko Carstens, Martin Schwidefsky,
	linux-wireless, netdev, linux-cifs, linux-kernel

On Thu, 2016-01-28 at 06:17 -0800, Kees Cook wrote:

> This changes several users of manual "on"/"off" parsing to use strtobool.

You should probably point out that it's a slight behaviour change for some
users. ie. parameters that previously *only* worked with "on"/"off", can now
also take 0/1/y/n etc.

But I don't think that's a show stopper.


> Signed-off-by: Kees Cook <keescook@chromium.org>
> Cc: x86@kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-s390@vger.kernel.org
> ---
>  arch/powerpc/kernel/rtasd.c                  | 10 +++-------
>  arch/powerpc/platforms/pseries/hotplug-cpu.c | 11 +++--------

Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)


cheers


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] param: convert some "on"/"off" users to strtobool
@ 2016-01-29  3:15     ` Michael Ellerman
  0 siblings, 0 replies; 14+ messages in thread
From: Michael Ellerman @ 2016-01-29  3:15 UTC (permalink / raw)
  To: Kees Cook, Andrew Morton
  Cc: x86-DgEjT+Ai2ygdnm+yROfE0A, linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-s390-u79uwXL29TY76Z2rM5mHXA, Joe Perches, Rasmus Villemoes,
	Daniel Borkmann, Amitkumar Karwar, Nishant Sarmukadam,
	Kalle Valo, Steve French, Heiko Carstens, Martin Schwidefsky,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Thu, 2016-01-28 at 06:17 -0800, Kees Cook wrote:

> This changes several users of manual "on"/"off" parsing to use strtobool.

You should probably point out that it's a slight behaviour change for some
users. ie. parameters that previously *only* worked with "on"/"off", can now
also take 0/1/y/n etc.

But I don't think that's a show stopper.


> Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Cc: x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
> Cc: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> Cc: linux-s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  arch/powerpc/kernel/rtasd.c                  | 10 +++-------
>  arch/powerpc/platforms/pseries/hotplug-cpu.c | 11 +++--------

Acked-by: Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org> (powerpc)


cheers

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] param: convert some "on"/"off" users to strtobool
@ 2016-01-29  3:15     ` Michael Ellerman
  0 siblings, 0 replies; 14+ messages in thread
From: Michael Ellerman @ 2016-01-29  3:15 UTC (permalink / raw)
  To: Kees Cook, Andrew Morton
  Cc: x86, linuxppc-dev, linux-s390, Joe Perches, Rasmus Villemoes,
	Daniel Borkmann, Amitkumar Karwar, Nishant Sarmukadam,
	Kalle Valo, Steve French, Heiko Carstens, Martin Schwidefsky,
	linux-wireless, netdev, linux-cifs, linux-kernel

On Thu, 2016-01-28 at 06:17 -0800, Kees Cook wrote:

> This changes several users of manual "on"/"off" parsing to use strtobool.

You should probably point out that it's a slight behaviour change for some
users. ie. parameters that previously *only* worked with "on"/"off", can now
also take 0/1/y/n etc.

But I don't think that's a show stopper.


> Signed-off-by: Kees Cook <keescook@chromium.org>
> Cc: x86@kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-s390@vger.kernel.org
> ---
>  arch/powerpc/kernel/rtasd.c                  | 10 +++-------
>  arch/powerpc/platforms/pseries/hotplug-cpu.c | 11 +++--------

Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)


cheers

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] lib: fix callers of strtobool to use char array
  2016-01-28 14:17   ` Kees Cook
  (?)
@ 2016-02-01 13:17   ` Andy Shevchenko
  2016-02-04 18:56     ` Kees Cook
  -1 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2016-02-01 13:17 UTC (permalink / raw)
  To: Kees Cook
  Cc: Andrew Morton, Amitkumar Karwar, Nishant Sarmukadam, Kalle Valo,
	Steve French, linux-cifs, Joe Perches, Rasmus Villemoes,
	Daniel Borkmann, Michael Ellerman, Heiko Carstens,
	Martin Schwidefsky, x86, linuxppc-dev, linux-s390,
	open list:TI WILINK WIRELES...,
	netdev, linux-kernel

On Thu, Jan 28, 2016 at 4:17 PM, Kees Cook <keescook@chromium.org> wrote:
> Some callers of strtobool were passing a pointer to unterminated strings.
> This fixes the issue and consolidates some logic in cifs.

My comments below.

First of all I don't think currently there is an issue in cifs, since
strbool checks only first character of the input string, or are you
talking about something else?

> diff --git a/drivers/net/wireless/marvell/mwifiex/debugfs.c b/drivers/net/wireless/marvell/mwifiex/debugfs.c
> index 0b9c580af988..76af60899c69 100644
> --- a/drivers/net/wireless/marvell/mwifiex/debugfs.c
> +++ b/drivers/net/wireless/marvell/mwifiex/debugfs.c
> @@ -880,13 +880,13 @@ mwifiex_reset_write(struct file *file,
>  {
>         struct mwifiex_private *priv = file->private_data;
>         struct mwifiex_adapter *adapter = priv->adapter;
> -       char cmd;
> +       char cmd[2] = { '\0' };
>         bool result;
>
> -       if (copy_from_user(&cmd, ubuf, sizeof(cmd)))
> +       if (copy_from_user(cmd, ubuf, sizeof(char)))
>                 return -EFAULT;
>
> -       if (strtobool(&cmd, &result))
> +       if (strtobool(cmd, &result))
>                 return -EINVAL;

Can we do strtobool_from_user() instead like kstrto*from_user() and
similar helpers are done?

>
>         if (!result)
> diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
> index 50b268483302..2f7ffcc9e364 100644
> --- a/fs/cifs/cifs_debug.c
> +++ b/fs/cifs/cifs_debug.c
> @@ -251,11 +251,29 @@ static const struct file_operations cifs_debug_data_proc_fops = {
>         .release        = single_release,
>  };
>
> +static int get_user_bool(const char __user *buffer, bool *store)
> +{
> +       char c[2] = { '\0' };
> +       bool bv;
> +       int rc;
> +
> +       rc = get_user(c[0], buffer);
> +       if (rc)
> +               return rc;
> +
> +       rc = strtobool(c, &bv);
> +       if (rc)
> +               return rc;
> +
> +       *store = bv;
> +
> +       return 0;
> +}
> +
>  #ifdef CONFIG_CIFS_STATS
>  static ssize_t cifs_stats_proc_write(struct file *file,
>                 const char __user *buffer, size_t count, loff_t *ppos)
>  {
> -       char c;
>         bool bv;
>         int rc;
>         struct list_head *tmp1, *tmp2, *tmp3;
> @@ -263,34 +281,32 @@ static ssize_t cifs_stats_proc_write(struct file *file,
>         struct cifs_ses *ses;
>         struct cifs_tcon *tcon;
>
> -       rc = get_user(c, buffer);
> +       rc = get_user_bool(buffer, &bv);
>         if (rc)
>                 return rc;
>
> -       if (strtobool(&c, &bv) == 0) {
>  #ifdef CONFIG_CIFS_STATS2

I would suggest to do a separate patch which just changes a pattern
and thus indentation without changing anything in functionality.

> -               atomic_set(&totBufAllocCount, 0);
> -               atomic_set(&totSmBufAllocCount, 0);
> +       atomic_set(&totBufAllocCount, 0);
> +       atomic_set(&totSmBufAllocCount, 0);
>  #endif /* CONFIG_CIFS_STATS2 */
> -               spin_lock(&cifs_tcp_ses_lock);
> -               list_for_each(tmp1, &cifs_tcp_ses_list) {
> -                       server = list_entry(tmp1, struct TCP_Server_Info,
> -                                           tcp_ses_list);
> -                       list_for_each(tmp2, &server->smb_ses_list) {
> -                               ses = list_entry(tmp2, struct cifs_ses,
> -                                                smb_ses_list);
> -                               list_for_each(tmp3, &ses->tcon_list) {
> -                                       tcon = list_entry(tmp3,
> -                                                         struct cifs_tcon,
> -                                                         tcon_list);
> -                                       atomic_set(&tcon->num_smbs_sent, 0);
> -                                       if (server->ops->clear_stats)
> -                                               server->ops->clear_stats(tcon);
> -                               }
> +       spin_lock(&cifs_tcp_ses_lock);
> +       list_for_each(tmp1, &cifs_tcp_ses_list) {
> +               server = list_entry(tmp1, struct TCP_Server_Info,
> +                                   tcp_ses_list);
> +               list_for_each(tmp2, &server->smb_ses_list) {
> +                       ses = list_entry(tmp2, struct cifs_ses,
> +                                        smb_ses_list);
> +                       list_for_each(tmp3, &ses->tcon_list) {
> +                               tcon = list_entry(tmp3,
> +                                                 struct cifs_tcon,
> +                                                 tcon_list);
> +                               atomic_set(&tcon->num_smbs_sent, 0);
> +                               if (server->ops->clear_stats)
> +                                       server->ops->clear_stats(tcon);
>                         }
>                 }
> -               spin_unlock(&cifs_tcp_ses_lock);
>         }
> +       spin_unlock(&cifs_tcp_ses_lock);
>
>         return count;
>  }
> @@ -433,17 +449,17 @@ static int cifsFYI_proc_open(struct inode *inode, struct file *file)
>  static ssize_t cifsFYI_proc_write(struct file *file, const char __user *buffer,
>                 size_t count, loff_t *ppos)
>  {
> -       char c;
> +       char c[2] = { '\0' };
>         bool bv;
>         int rc;
>
> -       rc = get_user(c, buffer);
> +       rc = get_user(c[0], buffer);
>         if (rc)
>                 return rc;
> -       if (strtobool(&c, &bv) == 0)
> +       if (strtobool(c, &bv) == 0)
>                 cifsFYI = bv;
> -       else if ((c > '1') && (c <= '9'))
> -               cifsFYI = (int) (c - '0'); /* see cifs_debug.h for meanings */
> +       else if ((c[0] > '1') && (c[0] <= '9'))
> +               cifsFYI = (int) (c[0] - '0'); /* see cifs_debug.h for meanings */
>
>         return count;
>  }
> @@ -471,20 +487,12 @@ static int cifs_linux_ext_proc_open(struct inode *inode, struct file *file)
>  static ssize_t cifs_linux_ext_proc_write(struct file *file,
>                 const char __user *buffer, size_t count, loff_t *ppos)
>  {
> -       char c;
> -       bool bv;
>         int rc;
>
> -       rc = get_user(c, buffer);
> +       rc = get_user_bool(buffer, &linuxExtEnabled);
>         if (rc)
>                 return rc;
>
> -       rc = strtobool(&c, &bv);
> -       if (rc)
> -               return rc;
> -
> -       linuxExtEnabled = bv;
> -
>         return count;
>  }
>
> @@ -511,20 +519,12 @@ static int cifs_lookup_cache_proc_open(struct inode *inode, struct file *file)
>  static ssize_t cifs_lookup_cache_proc_write(struct file *file,
>                 const char __user *buffer, size_t count, loff_t *ppos)
>  {
> -       char c;
> -       bool bv;
>         int rc;
>
> -       rc = get_user(c, buffer);
> +       rc = get_user_bool(buffer, &lookupCacheEnabled);
>         if (rc)
>                 return rc;
>
> -       rc = strtobool(&c, &bv);
> -       if (rc)
> -               return rc;
> -
> -       lookupCacheEnabled = bv;
> -
>         return count;
>  }
>
> @@ -551,20 +551,12 @@ static int traceSMB_proc_open(struct inode *inode, struct file *file)
>  static ssize_t traceSMB_proc_write(struct file *file, const char __user *buffer,
>                 size_t count, loff_t *ppos)
>  {
> -       char c;
> -       bool bv;
>         int rc;
>
> -       rc = get_user(c, buffer);
> +       rc = get_user_bool(buffer, &traceSMB);
>         if (rc)
>                 return rc;
>
> -       rc = strtobool(&c, &bv);
> -       if (rc)
> -               return rc;
> -
> -       traceSMB = bv;
> -
>         return count;
>  }
>
> @@ -622,7 +614,7 @@ static ssize_t cifs_security_flags_proc_write(struct file *file,
>         int rc;
>         unsigned int flags;
>         char flags_string[12];
> -       char c;
> +       char c[2] = { '\0' };
>         bool bv;
>
>         if ((count < 1) || (count > 11))
> @@ -635,11 +627,11 @@ static ssize_t cifs_security_flags_proc_write(struct file *file,
>
>         if (count < 3) {
>                 /* single char or single char followed by null */
> -               c = flags_string[0];
> -               if (strtobool(&c, &bv) == 0) {
> +               c[0] = flags_string[0];
> +               if (strtobool(c, &bv) == 0) {
>                         global_secflags = bv ? CIFSSEC_MAX : CIFSSEC_DEF;
>                         return count;
> -               } else if (!isdigit(c)) {
> +               } else if (!isdigit(c[0])) {
>                         cifs_dbg(VFS, "Invalid SecurityFlags: %s\n",
>                                         flags_string);
>                         return -EINVAL;
> diff --git a/fs/cifs/cifs_debug.h b/fs/cifs/cifs_debug.h
> index 66cf0f9fff89..c611ca2339d7 100644
> --- a/fs/cifs/cifs_debug.h
> +++ b/fs/cifs/cifs_debug.h
> @@ -25,7 +25,7 @@
>  void cifs_dump_mem(char *label, void *data, int length);
>  void cifs_dump_detail(void *);
>  void cifs_dump_mids(struct TCP_Server_Info *);
> -extern int traceSMB;           /* flag which enables the function below */
> +extern bool traceSMB;          /* flag which enables the function below */
>  void dump_smb(void *, int);
>  #define CIFS_INFO      0x01
>  #define CIFS_RC                0x02
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index c48ca13673e3..931b446f2a44 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -54,10 +54,10 @@
>  #endif
>
>  int cifsFYI = 0;
> -int traceSMB = 0;
> +bool traceSMB;
>  bool enable_oplocks = true;
> -unsigned int linuxExtEnabled = 1;
> -unsigned int lookupCacheEnabled = 1;
> +bool linuxExtEnabled = true;
> +bool lookupCacheEnabled = true;
>  unsigned int global_secflags = CIFSSEC_DEF;
>  /* unsigned int ntlmv2_support = 0; */
>  unsigned int sign_CIFS_PDUs = 1;
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index a25b2513f146..d21da9f05bae 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -1596,11 +1596,11 @@ GLOBAL_EXTERN atomic_t midCount;
>
>  /* Misc globals */
>  GLOBAL_EXTERN bool enable_oplocks; /* enable or disable oplocks */
> -GLOBAL_EXTERN unsigned int lookupCacheEnabled;
> +GLOBAL_EXTERN bool lookupCacheEnabled;
>  GLOBAL_EXTERN unsigned int global_secflags;    /* if on, session setup sent
>                                 with more secure ntlmssp2 challenge/resp */
>  GLOBAL_EXTERN unsigned int sign_CIFS_PDUs;  /* enable smb packet signing */
> -GLOBAL_EXTERN unsigned int linuxExtEnabled;/*enable Linux/Unix CIFS extensions*/
> +GLOBAL_EXTERN bool linuxExtEnabled;/*enable Linux/Unix CIFS extensions*/
>  GLOBAL_EXTERN unsigned int CIFSMaxBufSize;  /* max size not including hdr */
>  GLOBAL_EXTERN unsigned int cifs_min_rcv;    /* min size of big ntwrk buf pool */
>  GLOBAL_EXTERN unsigned int cifs_min_small;  /* min size of small buf pool */
> --
> 2.6.3
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] lib: fix callers of strtobool to use char array
  2016-02-01 13:17   ` Andy Shevchenko
@ 2016-02-04 18:56     ` Kees Cook
  0 siblings, 0 replies; 14+ messages in thread
From: Kees Cook @ 2016-02-04 18:56 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andrew Morton, Amitkumar Karwar, Nishant Sarmukadam, Kalle Valo,
	Steve French, linux-cifs, Joe Perches, Rasmus Villemoes,
	Daniel Borkmann, Michael Ellerman, Heiko Carstens,
	Martin Schwidefsky, x86, linuxppc-dev, linux-s390,
	open list:TI WILINK WIRELES...,
	netdev, linux-kernel

On Mon, Feb 1, 2016 at 5:17 AM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, Jan 28, 2016 at 4:17 PM, Kees Cook <keescook@chromium.org> wrote:
>> Some callers of strtobool were passing a pointer to unterminated strings.
>> This fixes the issue and consolidates some logic in cifs.
>
> My comments below.
>
> First of all I don't think currently there is an issue in cifs, since
> strbool checks only first character of the input string, or are you
> talking about something else?

Right, no, this is a fix before extending strtobool to parse the
second character in the string (for handling "on" and "off").

>> diff --git a/drivers/net/wireless/marvell/mwifiex/debugfs.c b/drivers/net/wireless/marvell/mwifiex/debugfs.c
>> index 0b9c580af988..76af60899c69 100644
>> --- a/drivers/net/wireless/marvell/mwifiex/debugfs.c
>> +++ b/drivers/net/wireless/marvell/mwifiex/debugfs.c
>> @@ -880,13 +880,13 @@ mwifiex_reset_write(struct file *file,
>>  {
>>         struct mwifiex_private *priv = file->private_data;
>>         struct mwifiex_adapter *adapter = priv->adapter;
>> -       char cmd;
>> +       char cmd[2] = { '\0' };
>>         bool result;
>>
>> -       if (copy_from_user(&cmd, ubuf, sizeof(cmd)))
>> +       if (copy_from_user(cmd, ubuf, sizeof(char)))
>>                 return -EFAULT;
>>
>> -       if (strtobool(&cmd, &result))
>> +       if (strtobool(cmd, &result))
>>                 return -EINVAL;
>
> Can we do strtobool_from_user() instead like kstrto*from_user() and
> similar helpers are done?

Yeah, that might clean this up a bit more. I will add it.

>>         if (!result)
>> diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
>> index 50b268483302..2f7ffcc9e364 100644
>> --- a/fs/cifs/cifs_debug.c
>> +++ b/fs/cifs/cifs_debug.c
>> @@ -251,11 +251,29 @@ static const struct file_operations cifs_debug_data_proc_fops = {
>>         .release        = single_release,
>>  };
>>
>> +static int get_user_bool(const char __user *buffer, bool *store)
>> +{
>> +       char c[2] = { '\0' };
>> +       bool bv;
>> +       int rc;
>> +
>> +       rc = get_user(c[0], buffer);
>> +       if (rc)
>> +               return rc;
>> +
>> +       rc = strtobool(c, &bv);
>> +       if (rc)
>> +               return rc;
>> +
>> +       *store = bv;
>> +
>> +       return 0;
>> +}
>> +
>>  #ifdef CONFIG_CIFS_STATS
>>  static ssize_t cifs_stats_proc_write(struct file *file,
>>                 const char __user *buffer, size_t count, loff_t *ppos)
>>  {
>> -       char c;
>>         bool bv;
>>         int rc;
>>         struct list_head *tmp1, *tmp2, *tmp3;
>> @@ -263,34 +281,32 @@ static ssize_t cifs_stats_proc_write(struct file *file,
>>         struct cifs_ses *ses;
>>         struct cifs_tcon *tcon;
>>
>> -       rc = get_user(c, buffer);
>> +       rc = get_user_bool(buffer, &bv);
>>         if (rc)
>>                 return rc;
>>
>> -       if (strtobool(&c, &bv) == 0) {
>>  #ifdef CONFIG_CIFS_STATS2
>
> I would suggest to do a separate patch which just changes a pattern
> and thus indentation without changing anything in functionality.

Okay, noted.

>> -               atomic_set(&totBufAllocCount, 0);
>> -               atomic_set(&totSmBufAllocCount, 0);
>> +       atomic_set(&totBufAllocCount, 0);
>> +       atomic_set(&totSmBufAllocCount, 0);
>>  #endif /* CONFIG_CIFS_STATS2 */
>> -               spin_lock(&cifs_tcp_ses_lock);
>> -               list_for_each(tmp1, &cifs_tcp_ses_list) {
>> -                       server = list_entry(tmp1, struct TCP_Server_Info,
>> -                                           tcp_ses_list);
>> -                       list_for_each(tmp2, &server->smb_ses_list) {
>> -                               ses = list_entry(tmp2, struct cifs_ses,
>> -                                                smb_ses_list);
>> -                               list_for_each(tmp3, &ses->tcon_list) {
>> -                                       tcon = list_entry(tmp3,
>> -                                                         struct cifs_tcon,
>> -                                                         tcon_list);
>> -                                       atomic_set(&tcon->num_smbs_sent, 0);
>> -                                       if (server->ops->clear_stats)
>> -                                               server->ops->clear_stats(tcon);
>> -                               }
>> +       spin_lock(&cifs_tcp_ses_lock);
>> +       list_for_each(tmp1, &cifs_tcp_ses_list) {
>> +               server = list_entry(tmp1, struct TCP_Server_Info,
>> +                                   tcp_ses_list);
>> +               list_for_each(tmp2, &server->smb_ses_list) {
>> +                       ses = list_entry(tmp2, struct cifs_ses,
>> +                                        smb_ses_list);
>> +                       list_for_each(tmp3, &ses->tcon_list) {
>> +                               tcon = list_entry(tmp3,
>> +                                                 struct cifs_tcon,
>> +                                                 tcon_list);
>> +                               atomic_set(&tcon->num_smbs_sent, 0);
>> +                               if (server->ops->clear_stats)
>> +                                       server->ops->clear_stats(tcon);
>>                         }
>>                 }
>> -               spin_unlock(&cifs_tcp_ses_lock);
>>         }
>> +       spin_unlock(&cifs_tcp_ses_lock);
>>
>>         return count;
>>  }
>> @@ -433,17 +449,17 @@ static int cifsFYI_proc_open(struct inode *inode, struct file *file)
>>  static ssize_t cifsFYI_proc_write(struct file *file, const char __user *buffer,
>>                 size_t count, loff_t *ppos)
>>  {
>> -       char c;
>> +       char c[2] = { '\0' };
>>         bool bv;
>>         int rc;
>>
>> -       rc = get_user(c, buffer);
>> +       rc = get_user(c[0], buffer);
>>         if (rc)
>>                 return rc;
>> -       if (strtobool(&c, &bv) == 0)
>> +       if (strtobool(c, &bv) == 0)
>>                 cifsFYI = bv;
>> -       else if ((c > '1') && (c <= '9'))
>> -               cifsFYI = (int) (c - '0'); /* see cifs_debug.h for meanings */
>> +       else if ((c[0] > '1') && (c[0] <= '9'))
>> +               cifsFYI = (int) (c[0] - '0'); /* see cifs_debug.h for meanings */
>>
>>         return count;
>>  }
>> @@ -471,20 +487,12 @@ static int cifs_linux_ext_proc_open(struct inode *inode, struct file *file)
>>  static ssize_t cifs_linux_ext_proc_write(struct file *file,
>>                 const char __user *buffer, size_t count, loff_t *ppos)
>>  {
>> -       char c;
>> -       bool bv;
>>         int rc;
>>
>> -       rc = get_user(c, buffer);
>> +       rc = get_user_bool(buffer, &linuxExtEnabled);
>>         if (rc)
>>                 return rc;
>>
>> -       rc = strtobool(&c, &bv);
>> -       if (rc)
>> -               return rc;
>> -
>> -       linuxExtEnabled = bv;
>> -
>>         return count;
>>  }
>>
>> @@ -511,20 +519,12 @@ static int cifs_lookup_cache_proc_open(struct inode *inode, struct file *file)
>>  static ssize_t cifs_lookup_cache_proc_write(struct file *file,
>>                 const char __user *buffer, size_t count, loff_t *ppos)
>>  {
>> -       char c;
>> -       bool bv;
>>         int rc;
>>
>> -       rc = get_user(c, buffer);
>> +       rc = get_user_bool(buffer, &lookupCacheEnabled);
>>         if (rc)
>>                 return rc;
>>
>> -       rc = strtobool(&c, &bv);
>> -       if (rc)
>> -               return rc;
>> -
>> -       lookupCacheEnabled = bv;
>> -
>>         return count;
>>  }
>>
>> @@ -551,20 +551,12 @@ static int traceSMB_proc_open(struct inode *inode, struct file *file)
>>  static ssize_t traceSMB_proc_write(struct file *file, const char __user *buffer,
>>                 size_t count, loff_t *ppos)
>>  {
>> -       char c;
>> -       bool bv;
>>         int rc;
>>
>> -       rc = get_user(c, buffer);
>> +       rc = get_user_bool(buffer, &traceSMB);
>>         if (rc)
>>                 return rc;
>>
>> -       rc = strtobool(&c, &bv);
>> -       if (rc)
>> -               return rc;
>> -
>> -       traceSMB = bv;
>> -
>>         return count;
>>  }
>>
>> @@ -622,7 +614,7 @@ static ssize_t cifs_security_flags_proc_write(struct file *file,
>>         int rc;
>>         unsigned int flags;
>>         char flags_string[12];
>> -       char c;
>> +       char c[2] = { '\0' };
>>         bool bv;
>>
>>         if ((count < 1) || (count > 11))
>> @@ -635,11 +627,11 @@ static ssize_t cifs_security_flags_proc_write(struct file *file,
>>
>>         if (count < 3) {
>>                 /* single char or single char followed by null */
>> -               c = flags_string[0];
>> -               if (strtobool(&c, &bv) == 0) {
>> +               c[0] = flags_string[0];
>> +               if (strtobool(c, &bv) == 0) {
>>                         global_secflags = bv ? CIFSSEC_MAX : CIFSSEC_DEF;
>>                         return count;
>> -               } else if (!isdigit(c)) {
>> +               } else if (!isdigit(c[0])) {
>>                         cifs_dbg(VFS, "Invalid SecurityFlags: %s\n",
>>                                         flags_string);
>>                         return -EINVAL;
>> diff --git a/fs/cifs/cifs_debug.h b/fs/cifs/cifs_debug.h
>> index 66cf0f9fff89..c611ca2339d7 100644
>> --- a/fs/cifs/cifs_debug.h
>> +++ b/fs/cifs/cifs_debug.h
>> @@ -25,7 +25,7 @@
>>  void cifs_dump_mem(char *label, void *data, int length);
>>  void cifs_dump_detail(void *);
>>  void cifs_dump_mids(struct TCP_Server_Info *);
>> -extern int traceSMB;           /* flag which enables the function below */
>> +extern bool traceSMB;          /* flag which enables the function below */
>>  void dump_smb(void *, int);
>>  #define CIFS_INFO      0x01
>>  #define CIFS_RC                0x02
>> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
>> index c48ca13673e3..931b446f2a44 100644
>> --- a/fs/cifs/cifsfs.c
>> +++ b/fs/cifs/cifsfs.c
>> @@ -54,10 +54,10 @@
>>  #endif
>>
>>  int cifsFYI = 0;
>> -int traceSMB = 0;
>> +bool traceSMB;
>>  bool enable_oplocks = true;
>> -unsigned int linuxExtEnabled = 1;
>> -unsigned int lookupCacheEnabled = 1;
>> +bool linuxExtEnabled = true;
>> +bool lookupCacheEnabled = true;
>>  unsigned int global_secflags = CIFSSEC_DEF;
>>  /* unsigned int ntlmv2_support = 0; */
>>  unsigned int sign_CIFS_PDUs = 1;
>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> index a25b2513f146..d21da9f05bae 100644
>> --- a/fs/cifs/cifsglob.h
>> +++ b/fs/cifs/cifsglob.h
>> @@ -1596,11 +1596,11 @@ GLOBAL_EXTERN atomic_t midCount;
>>
>>  /* Misc globals */
>>  GLOBAL_EXTERN bool enable_oplocks; /* enable or disable oplocks */
>> -GLOBAL_EXTERN unsigned int lookupCacheEnabled;
>> +GLOBAL_EXTERN bool lookupCacheEnabled;
>>  GLOBAL_EXTERN unsigned int global_secflags;    /* if on, session setup sent
>>                                 with more secure ntlmssp2 challenge/resp */
>>  GLOBAL_EXTERN unsigned int sign_CIFS_PDUs;  /* enable smb packet signing */
>> -GLOBAL_EXTERN unsigned int linuxExtEnabled;/*enable Linux/Unix CIFS extensions*/
>> +GLOBAL_EXTERN bool linuxExtEnabled;/*enable Linux/Unix CIFS extensions*/
>>  GLOBAL_EXTERN unsigned int CIFSMaxBufSize;  /* max size not including hdr */
>>  GLOBAL_EXTERN unsigned int cifs_min_rcv;    /* min size of big ntwrk buf pool */
>>  GLOBAL_EXTERN unsigned int cifs_min_small;  /* min size of small buf pool */
>> --
>> 2.6.3
>>
>
>
>
> --
> With Best Regards,
> Andy Shevchenko

Thanks for the review!

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-02-04 18:56 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-28 14:17 [PATCH 0/3] lib: add "on" and "off" to strtobool Kees Cook
2016-01-28 14:17 ` Kees Cook
2016-01-28 14:17 ` [PATCH 1/3] lib: fix callers of strtobool to use char array Kees Cook
2016-01-28 14:17   ` Kees Cook
2016-02-01 13:17   ` Andy Shevchenko
2016-02-04 18:56     ` Kees Cook
2016-01-28 14:17 ` [PATCH 2/3] lib: add "on" and "off" to strtobool Kees Cook
2016-01-28 14:17 ` [PATCH 3/3] param: convert some "on"/"off" users " Kees Cook
2016-01-28 14:17   ` Kees Cook
2016-01-28 15:27   ` Heiko Carstens
2016-01-28 15:27     ` Heiko Carstens
2016-01-29  3:15   ` Michael Ellerman
2016-01-29  3:15     ` Michael Ellerman
2016-01-29  3:15     ` Michael Ellerman

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.