linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Fix a double free bug in rsi_91x_deinit
@ 2019-08-19 22:02 Hui Peng
  2019-08-31 18:18 ` Guenter Roeck
  2019-09-03 13:55 ` [PATCH] rsi: fix a double free bug in rsi_91x_deinit() Kalle Valo
  0 siblings, 2 replies; 10+ messages in thread
From: Hui Peng @ 2019-08-19 22:02 UTC (permalink / raw)
  To: security
  Cc: Hui Peng, Mathias Payer, Kalle Valo, David S. Miller,
	linux-wireless, netdev, linux-kernel

`dev` (struct rsi_91x_usbdev *) field of adapter
(struct rsi_91x_usbdev *) is allocated  and initialized in
`rsi_init_usb_interface`. If any error is detected in information
read from the device side,  `rsi_init_usb_interface` will be
freed. However, in the higher level error handling code in
`rsi_probe`, if error is detected, `rsi_91x_deinit` is called
again, in which `dev` will be freed again, resulting double free.

This patch fixes the double free by removing the free operation on
`dev` in `rsi_init_usb_interface`, because `rsi_91x_deinit` is also
used in `rsi_disconnect`, in that code path, the `dev` field is not
 (and thus needs to be) freed.

This bug was found in v4.19, but is also present in the latest version
of kernel.

Reported-by: Hui Peng <benquike@gmail.com>
Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
Signed-off-by: Hui Peng <benquike@gmail.com>
---
 drivers/net/wireless/rsi/rsi_91x_usb.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_usb.c b/drivers/net/wireless/rsi/rsi_91x_usb.c
index c0a163e40402..ac917227f708 100644
--- a/drivers/net/wireless/rsi/rsi_91x_usb.c
+++ b/drivers/net/wireless/rsi/rsi_91x_usb.c
@@ -640,7 +640,6 @@ static int rsi_init_usb_interface(struct rsi_hw *adapter,
 	kfree(rsi_dev->tx_buffer);
 
 fail_eps:
-	kfree(rsi_dev);
 
 	return status;
 }
-- 
2.22.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix a double free bug in rsi_91x_deinit
  2019-08-19 22:02 [PATCH] Fix a double free bug in rsi_91x_deinit Hui Peng
@ 2019-08-31 18:18 ` Guenter Roeck
  2019-08-31 18:32   ` Hui Peng
  2019-09-01  8:03   ` Kalle Valo
  2019-09-03 13:55 ` [PATCH] rsi: fix a double free bug in rsi_91x_deinit() Kalle Valo
  1 sibling, 2 replies; 10+ messages in thread
From: Guenter Roeck @ 2019-08-31 18:18 UTC (permalink / raw)
  To: Hui Peng
  Cc: security, Mathias Payer, Kalle Valo, David S. Miller,
	linux-wireless, netdev, linux-kernel

On Mon, Aug 19, 2019 at 06:02:29PM -0400, Hui Peng wrote:
> `dev` (struct rsi_91x_usbdev *) field of adapter
> (struct rsi_91x_usbdev *) is allocated  and initialized in
> `rsi_init_usb_interface`. If any error is detected in information
> read from the device side,  `rsi_init_usb_interface` will be
> freed. However, in the higher level error handling code in
> `rsi_probe`, if error is detected, `rsi_91x_deinit` is called
> again, in which `dev` will be freed again, resulting double free.
> 
> This patch fixes the double free by removing the free operation on
> `dev` in `rsi_init_usb_interface`, because `rsi_91x_deinit` is also
> used in `rsi_disconnect`, in that code path, the `dev` field is not
>  (and thus needs to be) freed.
> 
> This bug was found in v4.19, but is also present in the latest version
> of kernel.
> 
> Reported-by: Hui Peng <benquike@gmail.com>
> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
> Signed-off-by: Hui Peng <benquike@gmail.com>

FWIW:

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

This patch is listed as fix for CVE-2019-15504, which has a CVSS 2.0 score
of 10.0 (high) and CVSS 3.0 score of 9.8 (critical).

Are there any plans to apply this patch to the upstream kernel anytime
soon ? If not, are there reasons to believe that the problem is not as
severe as its CVSS score may indicate ?

Thanks,
Guenter

> ---
>  drivers/net/wireless/rsi/rsi_91x_usb.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/rsi/rsi_91x_usb.c b/drivers/net/wireless/rsi/rsi_91x_usb.c
> index c0a163e40402..ac917227f708 100644
> --- a/drivers/net/wireless/rsi/rsi_91x_usb.c
> +++ b/drivers/net/wireless/rsi/rsi_91x_usb.c
> @@ -640,7 +640,6 @@ static int rsi_init_usb_interface(struct rsi_hw *adapter,
>  	kfree(rsi_dev->tx_buffer);
>  
>  fail_eps:
> -	kfree(rsi_dev);
>  
>  	return status;
>  }
> -- 
> 2.22.1
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix a double free bug in rsi_91x_deinit
  2019-08-31 18:18 ` Guenter Roeck
@ 2019-08-31 18:32   ` Hui Peng
  2019-09-01  8:03   ` Kalle Valo
  1 sibling, 0 replies; 10+ messages in thread
From: Hui Peng @ 2019-08-31 18:32 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: security, Mathias Payer, Kalle Valo, David S. Miller,
	linux-wireless, netdev, linux-kernel

On 8/31/19 2:18 PM, Guenter Roeck wrote:
> On Mon, Aug 19, 2019 at 06:02:29PM -0400, Hui Peng wrote:
>> `dev` (struct rsi_91x_usbdev *) field of adapter
>> (struct rsi_91x_usbdev *) is allocated  and initialized in
>> `rsi_init_usb_interface`. If any error is detected in information
>> read from the device side,  `rsi_init_usb_interface` will be
>> freed. However, in the higher level error handling code in
>> `rsi_probe`, if error is detected, `rsi_91x_deinit` is called
>> again, in which `dev` will be freed again, resulting double free.
>>
>> This patch fixes the double free by removing the free operation on
>> `dev` in `rsi_init_usb_interface`, because `rsi_91x_deinit` is also
>> used in `rsi_disconnect`, in that code path, the `dev` field is not
>>  (and thus needs to be) freed.
>>
>> This bug was found in v4.19, but is also present in the latest version
>> of kernel.
>>
>> Reported-by: Hui Peng <benquike@gmail.com>
>> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
>> Signed-off-by: Hui Peng <benquike@gmail.com>
> FWIW:
>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>
> This patch is listed as fix for CVE-2019-15504, which has a CVSS 2.0 score
> of 10.0 (high) and CVSS 3.0 score of 9.8 (critical).
>
> Are there any plans to apply this patch to the upstream kernel anytime
> soon ? If not, are there reasons to believe that the problem is not as
> severe as its CVSS score may indicate ?
This patch is also still under review.
> Thanks,
> Guenter
>
>> ---
>>  drivers/net/wireless/rsi/rsi_91x_usb.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/rsi/rsi_91x_usb.c b/drivers/net/wireless/rsi/rsi_91x_usb.c
>> index c0a163e40402..ac917227f708 100644
>> --- a/drivers/net/wireless/rsi/rsi_91x_usb.c
>> +++ b/drivers/net/wireless/rsi/rsi_91x_usb.c
>> @@ -640,7 +640,6 @@ static int rsi_init_usb_interface(struct rsi_hw *adapter,
>>  	kfree(rsi_dev->tx_buffer);
>>  
>>  fail_eps:
>> -	kfree(rsi_dev);
>>  
>>  	return status;
>>  }
>> -- 
>> 2.22.1
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix a double free bug in rsi_91x_deinit
  2019-08-31 18:18 ` Guenter Roeck
  2019-08-31 18:32   ` Hui Peng
@ 2019-09-01  8:03   ` Kalle Valo
  2019-09-01 14:08     ` Guenter Roeck
  1 sibling, 1 reply; 10+ messages in thread
From: Kalle Valo @ 2019-09-01  8:03 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Hui Peng, security, Mathias Payer, David S. Miller,
	linux-wireless, netdev, linux-kernel

Guenter Roeck <linux@roeck-us.net> writes:

> On Mon, Aug 19, 2019 at 06:02:29PM -0400, Hui Peng wrote:
>> `dev` (struct rsi_91x_usbdev *) field of adapter
>> (struct rsi_91x_usbdev *) is allocated  and initialized in
>> `rsi_init_usb_interface`. If any error is detected in information
>> read from the device side,  `rsi_init_usb_interface` will be
>> freed. However, in the higher level error handling code in
>> `rsi_probe`, if error is detected, `rsi_91x_deinit` is called
>> again, in which `dev` will be freed again, resulting double free.
>> 
>> This patch fixes the double free by removing the free operation on
>> `dev` in `rsi_init_usb_interface`, because `rsi_91x_deinit` is also
>> used in `rsi_disconnect`, in that code path, the `dev` field is not
>>  (and thus needs to be) freed.
>> 
>> This bug was found in v4.19, but is also present in the latest version
>> of kernel.
>> 
>> Reported-by: Hui Peng <benquike@gmail.com>
>> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
>> Signed-off-by: Hui Peng <benquike@gmail.com>
>
> FWIW:
>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>
> This patch is listed as fix for CVE-2019-15504, which has a CVSS 2.0 score
> of 10.0 (high) and CVSS 3.0 score of 9.8 (critical).

A double free in error path is considered as a critical CVE issue? I'm
very curious, why is that?

> Are there any plans to apply this patch to the upstream kernel anytime
> soon ?

I was on vacation last week and hence I have not been able to apply any
wireless patches. I should be able to catch up next week.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix a double free bug in rsi_91x_deinit
  2019-09-01  8:03   ` Kalle Valo
@ 2019-09-01 14:08     ` Guenter Roeck
  2019-09-02 18:47       ` Greg KH
  0 siblings, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2019-09-01 14:08 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Hui Peng, security, Mathias Payer, David S. Miller,
	linux-wireless, netdev, linux-kernel

On 9/1/19 1:03 AM, Kalle Valo wrote:
> Guenter Roeck <linux@roeck-us.net> writes:
> 
>> On Mon, Aug 19, 2019 at 06:02:29PM -0400, Hui Peng wrote:
>>> `dev` (struct rsi_91x_usbdev *) field of adapter
>>> (struct rsi_91x_usbdev *) is allocated  and initialized in
>>> `rsi_init_usb_interface`. If any error is detected in information
>>> read from the device side,  `rsi_init_usb_interface` will be
>>> freed. However, in the higher level error handling code in
>>> `rsi_probe`, if error is detected, `rsi_91x_deinit` is called
>>> again, in which `dev` will be freed again, resulting double free.
>>>
>>> This patch fixes the double free by removing the free operation on
>>> `dev` in `rsi_init_usb_interface`, because `rsi_91x_deinit` is also
>>> used in `rsi_disconnect`, in that code path, the `dev` field is not
>>>   (and thus needs to be) freed.
>>>
>>> This bug was found in v4.19, but is also present in the latest version
>>> of kernel.
>>>
>>> Reported-by: Hui Peng <benquike@gmail.com>
>>> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
>>> Signed-off-by: Hui Peng <benquike@gmail.com>
>>
>> FWIW:
>>
>> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>>
>> This patch is listed as fix for CVE-2019-15504, which has a CVSS 2.0 score
>> of 10.0 (high) and CVSS 3.0 score of 9.8 (critical).
> 
> A double free in error path is considered as a critical CVE issue? I'm
> very curious, why is that?
> 

You'd have to ask the people assigning CVSS scores. However, if the memory
was reallocated, that reallocated memory (which is still in use) is freed.
Then all kinds of bad things can happen.

Guenter

>> Are there any plans to apply this patch to the upstream kernel anytime
>> soon ?
> 
> I was on vacation last week and hence I have not been able to apply any
> wireless patches. I should be able to catch up next week.
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix a double free bug in rsi_91x_deinit
  2019-09-01 14:08     ` Guenter Roeck
@ 2019-09-02 18:47       ` Greg KH
  2019-09-02 19:32         ` Guenter Roeck
  0 siblings, 1 reply; 10+ messages in thread
From: Greg KH @ 2019-09-02 18:47 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Kalle Valo, Hui Peng, security, Mathias Payer, David S. Miller,
	linux-wireless, netdev, linux-kernel

On Sun, Sep 01, 2019 at 07:08:29AM -0700, Guenter Roeck wrote:
> On 9/1/19 1:03 AM, Kalle Valo wrote:
> > Guenter Roeck <linux@roeck-us.net> writes:
> > 
> > > On Mon, Aug 19, 2019 at 06:02:29PM -0400, Hui Peng wrote:
> > > > `dev` (struct rsi_91x_usbdev *) field of adapter
> > > > (struct rsi_91x_usbdev *) is allocated  and initialized in
> > > > `rsi_init_usb_interface`. If any error is detected in information
> > > > read from the device side,  `rsi_init_usb_interface` will be
> > > > freed. However, in the higher level error handling code in
> > > > `rsi_probe`, if error is detected, `rsi_91x_deinit` is called
> > > > again, in which `dev` will be freed again, resulting double free.
> > > > 
> > > > This patch fixes the double free by removing the free operation on
> > > > `dev` in `rsi_init_usb_interface`, because `rsi_91x_deinit` is also
> > > > used in `rsi_disconnect`, in that code path, the `dev` field is not
> > > >   (and thus needs to be) freed.
> > > > 
> > > > This bug was found in v4.19, but is also present in the latest version
> > > > of kernel.
> > > > 
> > > > Reported-by: Hui Peng <benquike@gmail.com>
> > > > Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
> > > > Signed-off-by: Hui Peng <benquike@gmail.com>
> > > 
> > > FWIW:
> > > 
> > > Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> > > 
> > > This patch is listed as fix for CVE-2019-15504, which has a CVSS 2.0 score
> > > of 10.0 (high) and CVSS 3.0 score of 9.8 (critical).
> > 
> > A double free in error path is considered as a critical CVE issue? I'm
> > very curious, why is that?
> > 
> 
> You'd have to ask the people assigning CVSS scores. However, if the memory
> was reallocated, that reallocated memory (which is still in use) is freed.
> Then all kinds of bad things can happen.

Yes, but moving from "bad things _can_ happen" to "bad things happen" in
an instance like this will be a tough task.  It also requires physical
access to the machine.

Anyway, that doesn't mean we shouldn't fix it, it's just that CVSS can
be crazy when it comes to kernel patches (i.e. almost all fixes should
be "critical"...)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix a double free bug in rsi_91x_deinit
  2019-09-02 18:47       ` Greg KH
@ 2019-09-02 19:32         ` Guenter Roeck
  2019-09-02 20:06           ` Greg KH
  0 siblings, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2019-09-02 19:32 UTC (permalink / raw)
  To: Greg KH
  Cc: Kalle Valo, Hui Peng, security, Mathias Payer, David S. Miller,
	linux-wireless, netdev, linux-kernel

On 9/2/19 11:47 AM, Greg KH wrote:
> On Sun, Sep 01, 2019 at 07:08:29AM -0700, Guenter Roeck wrote:
>> On 9/1/19 1:03 AM, Kalle Valo wrote:
>>> Guenter Roeck <linux@roeck-us.net> writes:
>>>
>>>> On Mon, Aug 19, 2019 at 06:02:29PM -0400, Hui Peng wrote:
>>>>> `dev` (struct rsi_91x_usbdev *) field of adapter
>>>>> (struct rsi_91x_usbdev *) is allocated  and initialized in
>>>>> `rsi_init_usb_interface`. If any error is detected in information
>>>>> read from the device side,  `rsi_init_usb_interface` will be
>>>>> freed. However, in the higher level error handling code in
>>>>> `rsi_probe`, if error is detected, `rsi_91x_deinit` is called
>>>>> again, in which `dev` will be freed again, resulting double free.
>>>>>
>>>>> This patch fixes the double free by removing the free operation on
>>>>> `dev` in `rsi_init_usb_interface`, because `rsi_91x_deinit` is also
>>>>> used in `rsi_disconnect`, in that code path, the `dev` field is not
>>>>>    (and thus needs to be) freed.
>>>>>
>>>>> This bug was found in v4.19, but is also present in the latest version
>>>>> of kernel.
>>>>>
>>>>> Reported-by: Hui Peng <benquike@gmail.com>
>>>>> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
>>>>> Signed-off-by: Hui Peng <benquike@gmail.com>
>>>>
>>>> FWIW:
>>>>
>>>> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>>>>
>>>> This patch is listed as fix for CVE-2019-15504, which has a CVSS 2.0 score
>>>> of 10.0 (high) and CVSS 3.0 score of 9.8 (critical).
>>>
>>> A double free in error path is considered as a critical CVE issue? I'm
>>> very curious, why is that?
>>>
>>
>> You'd have to ask the people assigning CVSS scores. However, if the memory
>> was reallocated, that reallocated memory (which is still in use) is freed.
>> Then all kinds of bad things can happen.
> 
> Yes, but moving from "bad things _can_ happen" to "bad things happen" in
> an instance like this will be a tough task.  It also requires physical
> access to the machine.
> 

Is this correct even with usbip enabled ?

> Anyway, that doesn't mean we shouldn't fix it, it's just that CVSS can
> be crazy when it comes to kernel patches (i.e. almost all fixes should
> be "critical"...)
> 

Not all of them, but probably too many. That is why I asked if the problem
is real. I _used_ to trust CVSS scores, but by now I am at least somewhat
suspicious - especially if a patch wasn't applied for a period of time,
like this series of usb patches.

Having said that, I am even more wary of double-free problems - those tend
to be notoriously difficult to debug. I'd rather have them out of my way,
even if they are unlikely to be seen in the real world (plus, Murphy
says that anything unlikely is going to happen almost immediately).

Guenter

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix a double free bug in rsi_91x_deinit
  2019-09-02 19:32         ` Guenter Roeck
@ 2019-09-02 20:06           ` Greg KH
  2019-09-03  0:35             ` Guenter Roeck
  0 siblings, 1 reply; 10+ messages in thread
From: Greg KH @ 2019-09-02 20:06 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Kalle Valo, Hui Peng, security, Mathias Payer, David S. Miller,
	linux-wireless, netdev, linux-kernel

On Mon, Sep 02, 2019 at 12:32:37PM -0700, Guenter Roeck wrote:
> On 9/2/19 11:47 AM, Greg KH wrote:
> > On Sun, Sep 01, 2019 at 07:08:29AM -0700, Guenter Roeck wrote:
> > > On 9/1/19 1:03 AM, Kalle Valo wrote:
> > > > Guenter Roeck <linux@roeck-us.net> writes:
> > > > 
> > > > > On Mon, Aug 19, 2019 at 06:02:29PM -0400, Hui Peng wrote:
> > > > > > `dev` (struct rsi_91x_usbdev *) field of adapter
> > > > > > (struct rsi_91x_usbdev *) is allocated  and initialized in
> > > > > > `rsi_init_usb_interface`. If any error is detected in information
> > > > > > read from the device side,  `rsi_init_usb_interface` will be
> > > > > > freed. However, in the higher level error handling code in
> > > > > > `rsi_probe`, if error is detected, `rsi_91x_deinit` is called
> > > > > > again, in which `dev` will be freed again, resulting double free.
> > > > > > 
> > > > > > This patch fixes the double free by removing the free operation on
> > > > > > `dev` in `rsi_init_usb_interface`, because `rsi_91x_deinit` is also
> > > > > > used in `rsi_disconnect`, in that code path, the `dev` field is not
> > > > > >    (and thus needs to be) freed.
> > > > > > 
> > > > > > This bug was found in v4.19, but is also present in the latest version
> > > > > > of kernel.
> > > > > > 
> > > > > > Reported-by: Hui Peng <benquike@gmail.com>
> > > > > > Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
> > > > > > Signed-off-by: Hui Peng <benquike@gmail.com>
> > > > > 
> > > > > FWIW:
> > > > > 
> > > > > Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> > > > > 
> > > > > This patch is listed as fix for CVE-2019-15504, which has a CVSS 2.0 score
> > > > > of 10.0 (high) and CVSS 3.0 score of 9.8 (critical).
> > > > 
> > > > A double free in error path is considered as a critical CVE issue? I'm
> > > > very curious, why is that?
> > > > 
> > > 
> > > You'd have to ask the people assigning CVSS scores. However, if the memory
> > > was reallocated, that reallocated memory (which is still in use) is freed.
> > > Then all kinds of bad things can happen.
> > 
> > Yes, but moving from "bad things _can_ happen" to "bad things happen" in
> > an instance like this will be a tough task.  It also requires physical
> > access to the machine.
> > 
> 
> Is this correct even with usbip enabled ?

Who has usbip enabled anywhere?  :)

I don't know if usbip can trigger this type of thing, maybe someone
needs to test that...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix a double free bug in rsi_91x_deinit
  2019-09-02 20:06           ` Greg KH
@ 2019-09-03  0:35             ` Guenter Roeck
  0 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2019-09-03  0:35 UTC (permalink / raw)
  To: Greg KH
  Cc: Kalle Valo, Hui Peng, security, Mathias Payer, David S. Miller,
	linux-wireless, netdev, linux-kernel

On 9/2/19 1:06 PM, Greg KH wrote:
> On Mon, Sep 02, 2019 at 12:32:37PM -0700, Guenter Roeck wrote:
>> On 9/2/19 11:47 AM, Greg KH wrote:
>>> On Sun, Sep 01, 2019 at 07:08:29AM -0700, Guenter Roeck wrote:
>>>> On 9/1/19 1:03 AM, Kalle Valo wrote:
>>>>> Guenter Roeck <linux@roeck-us.net> writes:
>>>>>
>>>>>> On Mon, Aug 19, 2019 at 06:02:29PM -0400, Hui Peng wrote:
>>>>>>> `dev` (struct rsi_91x_usbdev *) field of adapter
>>>>>>> (struct rsi_91x_usbdev *) is allocated  and initialized in
>>>>>>> `rsi_init_usb_interface`. If any error is detected in information
>>>>>>> read from the device side,  `rsi_init_usb_interface` will be
>>>>>>> freed. However, in the higher level error handling code in
>>>>>>> `rsi_probe`, if error is detected, `rsi_91x_deinit` is called
>>>>>>> again, in which `dev` will be freed again, resulting double free.
>>>>>>>
>>>>>>> This patch fixes the double free by removing the free operation on
>>>>>>> `dev` in `rsi_init_usb_interface`, because `rsi_91x_deinit` is also
>>>>>>> used in `rsi_disconnect`, in that code path, the `dev` field is not
>>>>>>>     (and thus needs to be) freed.
>>>>>>>
>>>>>>> This bug was found in v4.19, but is also present in the latest version
>>>>>>> of kernel.
>>>>>>>
>>>>>>> Reported-by: Hui Peng <benquike@gmail.com>
>>>>>>> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
>>>>>>> Signed-off-by: Hui Peng <benquike@gmail.com>
>>>>>>
>>>>>> FWIW:
>>>>>>
>>>>>> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>>>>>>
>>>>>> This patch is listed as fix for CVE-2019-15504, which has a CVSS 2.0 score
>>>>>> of 10.0 (high) and CVSS 3.0 score of 9.8 (critical).
>>>>>
>>>>> A double free in error path is considered as a critical CVE issue? I'm
>>>>> very curious, why is that?
>>>>>
>>>>
>>>> You'd have to ask the people assigning CVSS scores. However, if the memory
>>>> was reallocated, that reallocated memory (which is still in use) is freed.
>>>> Then all kinds of bad things can happen.
>>>
>>> Yes, but moving from "bad things _can_ happen" to "bad things happen" in
>>> an instance like this will be a tough task.  It also requires physical
>>> access to the machine.
>>>
>>
>> Is this correct even with usbip enabled ?
> 
> Who has usbip enabled anywhere?  :)
> 

It is enabled in Ubuntu, and it looks like it is enabled in Fedora as well.
It is disabled in Chrome OS. I didn't check other distributions.

> I don't know if usbip can trigger this type of thing, maybe someone
> needs to test that...
> 

I seemed to recall someone mentioning that it is possible to use usbip
for remote attacks. This is why I mentioned it. I don't recall details,
though, and I don't know if it is really possible and to what extent.

Guenter

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] rsi: fix a double free bug in rsi_91x_deinit()
  2019-08-19 22:02 [PATCH] Fix a double free bug in rsi_91x_deinit Hui Peng
  2019-08-31 18:18 ` Guenter Roeck
@ 2019-09-03 13:55 ` Kalle Valo
  1 sibling, 0 replies; 10+ messages in thread
From: Kalle Valo @ 2019-09-03 13:55 UTC (permalink / raw)
  To: Hui Peng
  Cc: security, Hui Peng, Mathias Payer, David S. Miller,
	linux-wireless, netdev, linux-kernel

Hui Peng <benquike@gmail.com> wrote:

> `dev` (struct rsi_91x_usbdev *) field of adapter
> (struct rsi_91x_usbdev *) is allocated  and initialized in
> `rsi_init_usb_interface`. If any error is detected in information
> read from the device side,  `rsi_init_usb_interface` will be
> freed. However, in the higher level error handling code in
> `rsi_probe`, if error is detected, `rsi_91x_deinit` is called
> again, in which `dev` will be freed again, resulting double free.
> 
> This patch fixes the double free by removing the free operation on
> `dev` in `rsi_init_usb_interface`, because `rsi_91x_deinit` is also
> used in `rsi_disconnect`, in that code path, the `dev` field is not
>  (and thus needs to be) freed.
> 
> This bug was found in v4.19, but is also present in the latest version
> of kernel. Fixes CVE-2019-15504.
> 
> Reported-by: Hui Peng <benquike@gmail.com>
> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
> Signed-off-by: Hui Peng <benquike@gmail.com>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Patch applied to wireless-drivers.git, thanks.

8b51dc729147 rsi: fix a double free bug in rsi_91x_deinit()

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

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-09-03 13:55 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-19 22:02 [PATCH] Fix a double free bug in rsi_91x_deinit Hui Peng
2019-08-31 18:18 ` Guenter Roeck
2019-08-31 18:32   ` Hui Peng
2019-09-01  8:03   ` Kalle Valo
2019-09-01 14:08     ` Guenter Roeck
2019-09-02 18:47       ` Greg KH
2019-09-02 19:32         ` Guenter Roeck
2019-09-02 20:06           ` Greg KH
2019-09-03  0:35             ` Guenter Roeck
2019-09-03 13:55 ` [PATCH] rsi: fix a double free bug in rsi_91x_deinit() Kalle Valo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).