[PATCH] iwlwifi: pcie: Fixed integer overflow in iwl_write_to_user_buf

[PATCH] iwlwifi: pcie: Fixed integer overflow in iwl_write_to_user_buf

[Hyunwoo Kim]

An integer overflow occurs in the iwl_write_to_user_buf() function,
   which is called by the iwl_dbgfs_monitor_data_read() function.

static bool iwl_write_to_user_buf(char __user *user_buf, ssize_t count,
				  void *buf, ssize_t *size,
				  ssize_t *bytes_copied)
{
	int buf_size_left = count - *bytes_copied;

	buf_size_left = buf_size_left - (buf_size_left % sizeof(u32));
	if (*size > buf_size_left)
		*size = buf_size_left;

If the user passes a SIZE_MAX value to the "ssize_t count" parameter,
   the ssize_t count parameter is assigned to "int buf_size_left".
Then compare "*size" with "buf_size_left" . Here, "buf_size_left" is a
negative number, so "*size" is assigned "buf_size_left" and goes into
the third argument of the copy_to_user function, causing a heap overflow.

This is not a security vulnerability because iwl_dbgfs_monitor_data_read()
is a debugfs operation with 0400 privileges.

Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
 drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index bd50f52a1aad..fded5d305b11 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -2854,7 +2854,7 @@ static bool iwl_write_to_user_buf(char __user *user_buf, ssize_t count,
 				  void *buf, ssize_t *size,
 				  ssize_t *bytes_copied)
 {
-	int buf_size_left = count - *bytes_copied;
+	ssize_t buf_size_left = count - *bytes_copied;
 
 	buf_size_left = buf_size_left - (buf_size_left % sizeof(u32));
 	if (*size > buf_size_left)
-- 
2.25.1

Dear all,

I submitted this patch 11 days ago.

Can I get feedback on this patch?

Regards,
Hyunwoo Kim.

Re: [PATCH] iwlwifi: pcie: Fixed integer overflow in iwl_write_to_user_buf

[Kalle Valo]

> I submitted this patch 11 days ago.
>
> Can I get feedback on this patch?

11 days is not that long, we have other things to do as well. Please
don't resend a patch, that just increases our workload. Instead comment
on your original patch and ask for review, but please wait more than 11
days before commenting.

Your original patch is in patchwork so it is in the queue:

https://patchwork.kernel.org/project/linux-wireless/patch/20220614173352.GA588327@ubuntu/

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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

[PATCH] iwlwifi: pcie: Fixed integer overflow in iwl_write_to_user_buf

[Hyunwoo Kim]

An integer overflow occurs in the iwl_write_to_user_buf() function,
   which is called by the iwl_dbgfs_monitor_data_read() function.

static bool iwl_write_to_user_buf(char __user *user_buf, ssize_t count,
				  void *buf, ssize_t *size,
				  ssize_t *bytes_copied)
{
	int buf_size_left = count - *bytes_copied;

	buf_size_left = buf_size_left - (buf_size_left % sizeof(u32));
	if (*size > buf_size_left)
		*size = buf_size_left;

If the user passes a SIZE_MAX value to the "ssize_t count" parameter,
   the ssize_t count parameter is assigned to "int buf_size_left".
Then compare "*size" with "buf_size_left" . Here, "buf_size_left" is a
negative number, so "*size" is assigned "buf_size_left" and goes into
the third argument of the copy_to_user function, causing a heap overflow.

This is not a security vulnerability because iwl_dbgfs_monitor_data_read()
is a debugfs operation with 0400 privileges.

Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
 drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index bd50f52a1aad..fded5d305b11 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -2854,7 +2854,7 @@ static bool iwl_write_to_user_buf(char __user *user_buf, ssize_t count,
 				  void *buf, ssize_t *size,
 				  ssize_t *bytes_copied)
 {
-	int buf_size_left = count - *bytes_copied;
+	ssize_t buf_size_left = count - *bytes_copied;
 
 	buf_size_left = buf_size_left - (buf_size_left % sizeof(u32));
 	if (*size > buf_size_left)
-- 
2.25.1

Re: [PATCH] iwlwifi: pcie: Fixed integer overflow in iwl_write_to_user_buf

[Johannes Berg]

On Tue, 2022-06-14 at 10:33 -0700, Hyunwoo Kim wrote:
> An integer overflow occurs in the iwl_write_to_user_buf() function,
>    which is called by the iwl_dbgfs_monitor_data_read() function.
> 

Out of curiosity, how did you find this?

johannes

Re: [PATCH] iwlwifi: pcie: Fixed integer overflow in iwl_write_to_user_buf

[Hyunwoo Kim]

On Wed, Jun 15, 2022 at 09:06:51AM +0200, Johannes Berg wrote:
> On Tue, 2022-06-14 at 10:33 -0700, Hyunwoo Kim wrote:
> > An integer overflow occurs in the iwl_write_to_user_buf() function,
> >    which is called by the iwl_dbgfs_monitor_data_read() function.
> > 
> 
> Out of curiosity, how did you find this?

I found it while analyzing several device drivers as a personal hobby.


I also want to ask you one question.
While analyzing several device drivers, I found several such integer overflow 
or race condition problems, and made and submitted a patch.

https://marc.info/?l=linux-fbdev&m=165497564701256&w=2
https://www.spinics.net/lists/linux-efi/msg24884.html

However, there is no response whether this patch has been accepted or rejected. 
In this case, do I have to send an email to the higher level maintainer? Or do I have to wait?

Thanks,
Hyunwoo Kim