[PATCH] ath10k: fix out of bounds access to local buffer

[PATCH] ath10k: fix out of bounds access to local buffer

[Michael Mera]

During write to debugfs file simulate_fw_crash, fixed-size local buffer
'buf' is accessed and modified at index 'count-1', where 'count' is the
size of the write (so potentially out of bounds).
This patch fixes this problem.

Signed-off-by: Michael Mera <dev@michaelmera.com>
---
 drivers/net/wireless/ath/ath10k/debug.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index fb0ade3adb07..7f3c17e55693 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -628,17 +628,21 @@ static ssize_t ath10k_write_simulate_fw_crash(struct file *file,
 					      size_t count, loff_t *ppos)
 {
 	struct ath10k *ar = file->private_data;
-	char buf[32];
+	char buf[32] = {0};
+	ssize_t rc;
 	int ret;
 
-	simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf, count);
+	/* filter partial writes and invalid commands */
+	if (*ppos != 0 || count >= sizeof(buf) || count == 0)
+		return -EINVAL;
 
-	/* make sure that buf is null terminated */
-	buf[sizeof(buf) - 1] = 0;
+	rc = simple_write_to_buffer(buf, sizeof(buf)-1, ppos, user_buf, count);
+	if (rc < 0)
+		return rc;
 
 	/* drop the possible '\n' from the end */
-	if (buf[count - 1] == '\n')
-		buf[count - 1] = 0;
+	if (buf[*ppos - 1] == '\n')
+		buf[*ppos - 1] = '\0';
 
 	mutex_lock(&ar->conf_mutex);
 
-- 
2.9.3

Re: [PATCH] ath10k: fix out of bounds access to local buffer

[Marcin Rokicki]

Hi,

Please send again to ath10k@lists.infradead.org with cc
linux-wireless@vger.kernel.org

Thanks.

2017-04-24 7:39 GMT+02:00 Michael Mera <dev@michaelmera.com>:
> During write to debugfs file simulate_fw_crash, fixed-size local buffer
> 'buf' is accessed and modified at index 'count-1', where 'count' is the
> size of the write (so potentially out of bounds).
> This patch fixes this problem.
>
> Signed-off-by: Michael Mera <dev@michaelmera.com>
> ---
>  drivers/net/wireless/ath/ath10k/debug.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
> index fb0ade3adb07..7f3c17e55693 100644
> --- a/drivers/net/wireless/ath/ath10k/debug.c
> +++ b/drivers/net/wireless/ath/ath10k/debug.c
> @@ -628,17 +628,21 @@ static ssize_t ath10k_write_simulate_fw_crash(struct file *file,
>                                               size_t count, loff_t *ppos)
>  {
>         struct ath10k *ar = file->private_data;
> -       char buf[32];
> +       char buf[32] = {0};
> +       ssize_t rc;
>         int ret;
>
> -       simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf, count);
> +       /* filter partial writes and invalid commands */
> +       if (*ppos != 0 || count >= sizeof(buf) || count == 0)
> +               return -EINVAL;
>
> -       /* make sure that buf is null terminated */
> -       buf[sizeof(buf) - 1] = 0;
> +       rc = simple_write_to_buffer(buf, sizeof(buf)-1, ppos, user_buf, count);
> +       if (rc < 0)
> +               return rc;
>
>         /* drop the possible '\n' from the end */
> -       if (buf[count - 1] == '\n')
> -               buf[count - 1] = 0;
> +       if (buf[*ppos - 1] == '\n')
> +               buf[*ppos - 1] = '\0';
>
>         mutex_lock(&ar->conf_mutex);
>
> --
> 2.9.3
>

Re: [PATCH] ath10k: fix out of bounds access to local buffer

[Michael Mera]

Marcin Rokicki <marcin.rokicki@gmail.com> writes:
> Please send again to ath10k@lists.infradead.org with cc
> linux-wireless@vger.kernel.org
>

Sorry for the mistake. Resent as requested.

Just for the record, I followed instructions at:
  https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches?s[]=submit#who_to_address

So, maybe this needs to be updated to reflect the ath10k case.

Thanks,
Michael Mera

Re: [PATCH] ath10k: fix out of bounds access to local buffer

[Marcin Rokicki]

2017-04-24 9:41 GMT+02:00 Michael Mera <dev@michaelmera.com>:
> Marcin Rokicki <marcin.rokicki@gmail.com> writes:
>> Please send again to ath10k@lists.infradead.org with cc
>> linux-wireless@vger.kernel.org
>>
>
> Sorry for the mistake. Resent as requested.

No problem.
>
> Just for the record, I followed instructions at:
>   https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches?s[]=submit#who_to_address
>
> So, maybe this needs to be updated to reflect the ath10k case.

Check this page:
https://wireless.wiki.kernel.org/en/users/drivers/ath10k/sources
>
> Thanks,
> Michael Mera

Re: [PATCH] ath10k: fix out of bounds access to local buffer

[Kalle Valo]

Michael Mera <dev@michaelmera.com> writes:

> Marcin Rokicki <marcin.rokicki@gmail.com> writes:
>> Please send again to ath10k@lists.infradead.org with cc
>> linux-wireless@vger.kernel.org
>>
>
> Sorry for the mistake. Resent as requested.
>
> Just for the record, I followed instructions at:
>   https://wireless.wiki.kernel.org/en/developers/documentation/submitting=
patches?s[]=3Dsubmit#who_to_address
>
> So, maybe this needs to be updated to reflect the ath10k case.

I added a link to the corresponding ath10k page. BTW, it's a wiki so
everyone are free to improve the instructions on their own.

--=20
Kalle Valo=

Re: [PATCH] ath10k: fix out of bounds access to local buffer

[Michael Mera]

Kalle Valo <kvalo@qca.qualcomm.com> writes:
> I added a link to the corresponding ath10k page. BTW, it's a wiki so
> everyone are free to improve the instructions on their own.

As it was my first contribution, I didn't feel like editing things
without some sort of approval first. I will try to be more audacious
next time.

Thank you very much,
Michael Mera