diff mbox series

acpi: Fix use-after-free in acpi_ipmi.c

Message ID 1606222266-11685-1-git-send-email-tangyouling@loongson.cn
State New
Headers show
Series acpi: Fix use-after-free in acpi_ipmi.c | expand

Commit Message

Youling Tang Nov. 24, 2020, 12:51 p.m. UTC
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(-)

Comments

Rafael J. Wysocki Nov. 25, 2020, 3:53 p.m. UTC | #1
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

>
Youling Tang Nov. 26, 2020, 1:16 a.m. UTC | #2
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

>>
diff mbox series

Patch

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)