[PATCH] ath9k: fix tx99 potential info leak

[PATCH] ath9k: fix tx99 potential info leak

[miaoqing]

From: Miaoqing Pan <miaoqing@codeaurora.org>

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 <miaoqing@codeaurora.org>
---
 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

Re: [PATCH] ath9k: fix tx99 potential info leak

[Christoph Böhmwalder]

Am 27. September 2017 03:13:34 MESZ schrieb miaoqing@codeaurora.org:
>From: Miaoqing Pan <miaoqing@codeaurora.org>
>
>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 <miaoqing@codeaurora.org>
>---
> 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';
>+

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

Re: [PATCH] ath9k: fix tx99 potential info leak

[Christoph Böhmwalder]

[-- Attachment #1.1: Type: text/plain, Size: 505 bytes --]


>>
>> +	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.

>> Signed-off-by: Miaoqing Pan <miaoqing@codeaurora.org>

Reviewed-by: Christoph Böhmwalder <christoph@boehmwalder.at>

-- 
Regards,
Christoph


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: ath9k: fix tx99 potential info leak

[Kalle Valo]

miaoqing pan <miaoqing@codeaurora.org> 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 <miaoqing@codeaurora.org>
> Reviewed-by: Christoph Böhmwalder <christoph@boehmwalder.at>
> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>

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