[PATCH] ath9k: fix tx99 potential info leak

From: Miaoqing Pan <redacted>

When the user sets count to zero the string buffer would remain
completely uninitialized which causes the kernel to parse its
own stack data, potentially leading to an info leak. In addition
to that, the string might be not terminated properly when the
user data does not contain a 0-terminator.

Signed-off-by: Miaoqing Pan <redacted>
---
 drivers/net/wireless/ath/ath9k/tx99.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/tx99.c b/drivers/net/wireless/ath/ath9k/tx99.c
index 49ed1af..fe3a826 100644
--- a/drivers/net/wireless/ath/ath9k/tx99.c
+++ b/drivers/net/wireless/ath/ath9k/tx99.c

@@ -179,6 +179,9 @@ static ssize_t write_file_tx99(struct file *file, const char __user *user_buf,
 	ssize_t len;
 	int r;
 
+	if (count < 1)
+		return -EINVAL;
+
 	if (sc->cur_chan->nvifs > 1)
 		return -EOPNOTSUPP;
 

@@ -186,6 +189,8 @@ static ssize_t write_file_tx99(struct file *file, const char __user *user_buf,
 	if (copy_from_user(buf, user_buf, len))
 		return -EFAULT;
 
+	buf[len] = '\0';
+
 	if (strtobool(buf, &start))
 		return -EINVAL;
 

-- 
1.9.1

Am 27. September 2017 03:13:34 MESZ schrieb miaoqing@codeaurora.org:

I think it would be more appropriate here to check if buf[len] == '\0' and return an error otherwise.

	if (strtobool(buf, &start))
		return -EINVAL;

-- 
1.9.1

--
Regards,
Christoph

quoted

+	buf[len] = '\0';
+

I think it would be more appropriate here to check if buf[len] == '\0' and return an error otherwise.

Nevermind, I just had a closer look and I actually think your approach
is fine. I hadn't considered the possibility of someone deliberately
passing a non-null-terminated string with a specific length.

quoted

Signed-off-by: Miaoqing Pan <redacted>

Reviewed-by: Christoph Böhmwalder <redacted>

-- 
Regards,
Christoph

miaoqing pan [off-list ref] wrote:

When the user sets count to zero the string buffer would remain
completely uninitialized which causes the kernel to parse its
own stack data, potentially leading to an info leak. In addition
to that, the string might be not terminated properly when the
user data does not contain a 0-terminator.

Signed-off-by: Miaoqing Pan <redacted>
Reviewed-by: Christoph Böhmwalder <redacted>
Signed-off-by: Kalle Valo <redacted>

Patch applied to ath-next branch of ath.git, thanks.

ee0a47186e2f ath9k: fix tx99 potential info leak

-- 
https://patchwork.kernel.org/patch/9972889/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches