[PATCH] acpi: Fix use-after-free in acpi_ipmi.c

[PATCH] acpi: Fix use-after-free in acpi_ipmi.c

[Youling Tang]

kfree() has been called inside put_device so anther kfree would cause a
use-after-free bug.

Signed-off-by: Youling Tang <tangyouling@loongson.cn>
---
 drivers/acpi/acpi_ipmi.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c
index 9d6c0fc..72902b6 100644
--- a/drivers/acpi/acpi_ipmi.c
+++ b/drivers/acpi/acpi_ipmi.c
@@ -130,7 +130,6 @@ ipmi_dev_alloc(int iface, struct device *dev, acpi_handle handle)
 			       ipmi_device, &user);
 	if (err) {
 		put_device(dev);
-		kfree(ipmi_device);
 		return NULL;
 	}
 	ipmi_device->user_interface = user;
@@ -142,7 +141,6 @@ static void ipmi_dev_release(struct acpi_ipmi_device *ipmi_device)
 {
 	ipmi_destroy_user(ipmi_device->user_interface);
 	put_device(ipmi_device->dev);
-	kfree(ipmi_device);
 }
 
 static void ipmi_dev_release_kref(struct kref *kref)
-- 
2.1.0

Re: [PATCH] acpi: Fix use-after-free in acpi_ipmi.c

[Rafael J. Wysocki]

On Tue, Nov 24, 2020 at 1:51 PM Youling Tang <tangyouling@loongson.cn> wrote:
>
> kfree() has been called inside put_device so anther kfree would cause a
> use-after-free bug.
>
> Signed-off-by: Youling Tang <tangyouling@loongson.cn>
> ---
>  drivers/acpi/acpi_ipmi.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c
> index 9d6c0fc..72902b6 100644
> --- a/drivers/acpi/acpi_ipmi.c
> +++ b/drivers/acpi/acpi_ipmi.c
> @@ -130,7 +130,6 @@ ipmi_dev_alloc(int iface, struct device *dev, acpi_handle handle)
>                                ipmi_device, &user);
>         if (err) {
>                 put_device(dev);
> -               kfree(ipmi_device);

dev doesn't point to the same object in memory as ipmi_device, though,
if I'm not mistaken.

Please double check that and resend the patch if you are sure that it
is correct.

>                 return NULL;
>         }
>         ipmi_device->user_interface = user;
> @@ -142,7 +141,6 @@ static void ipmi_dev_release(struct acpi_ipmi_device *ipmi_device)
>  {
>         ipmi_destroy_user(ipmi_device->user_interface);
>         put_device(ipmi_device->dev);
> -       kfree(ipmi_device);
>  }
>
>  static void ipmi_dev_release_kref(struct kref *kref)
> --
> 2.1.0
>

Re: [PATCH] acpi: Fix use-after-free in acpi_ipmi.c

[Youling Tang]

Hi，
On 11/25/2020 11:53 PM, Rafael J. Wysocki wrote:
> On Tue, Nov 24, 2020 at 1:51 PM Youling Tang <tangyouling@loongson.cn> wrote:
>> kfree() has been called inside put_device so anther kfree would cause a
>> use-after-free bug.
>>
>> Signed-off-by: Youling Tang <tangyouling@loongson.cn>
>> ---
>>   drivers/acpi/acpi_ipmi.c | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c
>> index 9d6c0fc..72902b6 100644
>> --- a/drivers/acpi/acpi_ipmi.c
>> +++ b/drivers/acpi/acpi_ipmi.c
>> @@ -130,7 +130,6 @@ ipmi_dev_alloc(int iface, struct device *dev, acpi_handle handle)
>>                                 ipmi_device, &user);
>>          if (err) {
>>                  put_device(dev);
>> -               kfree(ipmi_device);
> dev doesn't point to the same object in memory as ipmi_device, though,
> if I'm not mistaken.
>
> Please double check that and resend the patch if you are sure that it
> is correct.
You're right, dev really doesn't point to the same memory object
as ipmi_device. I'll send v2 later.

Thanks,
Youling.
>>                  return NULL;
>>          }
>>          ipmi_device->user_interface = user;
>> @@ -142,7 +141,6 @@ static void ipmi_dev_release(struct acpi_ipmi_device *ipmi_device)
>>   {
>>          ipmi_destroy_user(ipmi_device->user_interface);
>>          put_device(ipmi_device->dev);
>> -       kfree(ipmi_device);
>>   }
>>
>>   static void ipmi_dev_release_kref(struct kref *kref)
>> --
>> 2.1.0
>>