2022-10-13 13:20:48

by Yang Yingliang

Subject: [PATCH] platform/loongarch: laptop: fix possible UAF in generic_acpi_laptop_init()

Current the return value of 'sub_driver->init' is not checked,
if sparse_keymap_setup() called in the init function fails,
'generic_inputdev' is freed, then it willl lead a UAF when
using it in generic_acpi_laptop_init(). Fix it by checking
return value. Set generic_inputdev to NULL after free to avoid
double free it.

Fixes: 6246ed09111f ("LoongArch: Add ACPI-based generic laptop driver")
Signed-off-by: Yang Yingliang <[email protected]>
---
drivers/platform/loongarch/loongson-laptop.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/loongarch/loongson-laptop.c b/drivers/platform/loongarch/loongson-laptop.c
index f0166ad5d2c2..a665fd1042ac 100644
--- a/drivers/platform/loongarch/loongson-laptop.c
+++ b/drivers/platform/loongarch/loongson-laptop.c
@@ -448,6 +448,7 @@ static int __init event_init(struct generic_sub_driver *sub_driver)
if (ret < 0) {
pr_err("Failed to setup input device keymap\n");
input_free_device(generic_inputdev);
+ generic_inputdev = NULL;

return ret;
}
@@ -502,8 +503,11 @@ static int __init generic_subdriver_init(struct generic_sub_driver *sub_driver)
if (ret)
return -EINVAL;

- if (sub_driver->init)
- sub_driver->init(sub_driver);
+ if (sub_driver->init) {
+ ret = sub_driver->init(sub_driver);
+ if (ret)
+ goto err_out;
+ }

if (sub_driver->notify) {
ret = setup_acpi_notify(sub_driver);
--
2.25.1

2022-10-13 15:58:27

by Huacai Chen

Subject: Re: [PATCH] platform/loongarch: laptop: fix possible UAF in generic_acpi_laptop_init()

Hi, Yingliang,

Thank you for your patches, but could you please merge the two trivial
patches to a single one?

Huacai

On Thu, Oct 13, 2022 at 9:12 PM Yang Yingliang <[email protected]> wrote:
>
> Current the return value of 'sub_driver->init' is not checked,
> if sparse_keymap_setup() called in the init function fails,
> 'generic_inputdev' is freed, then it willl lead a UAF when
> using it in generic_acpi_laptop_init(). Fix it by checking
> return value. Set generic_inputdev to NULL after free to avoid
> double free it.
>
> Fixes: 6246ed09111f ("LoongArch: Add ACPI-based generic laptop driver")
> Signed-off-by: Yang Yingliang <[email protected]>
> ---
> drivers/platform/loongarch/loongson-laptop.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/loongarch/loongson-laptop.c b/drivers/platform/loongarch/loongson-laptop.c
> index f0166ad5d2c2..a665fd1042ac 100644
> --- a/drivers/platform/loongarch/loongson-laptop.c
> +++ b/drivers/platform/loongarch/loongson-laptop.c
> @@ -448,6 +448,7 @@ static int __init event_init(struct generic_sub_driver *sub_driver)
> if (ret < 0) {
> pr_err("Failed to setup input device keymap\n");
> input_free_device(generic_inputdev);
> + generic_inputdev = NULL;
>
> return ret;
> }
> @@ -502,8 +503,11 @@ static int __init generic_subdriver_init(struct generic_sub_driver *sub_driver)
> if (ret)
> return -EINVAL;
>
> - if (sub_driver->init)
> - sub_driver->init(sub_driver);
> + if (sub_driver->init) {
> + ret = sub_driver->init(sub_driver);
> + if (ret)
> + goto err_out;
> + }
>
> if (sub_driver->notify) {
> ret = setup_acpi_notify(sub_driver);
> --
> 2.25.1
>

2022-10-14 01:59:13

by Yang Yingliang

Subject: Re: [PATCH] platform/loongarch: laptop: fix possible UAF in generic_acpi_laptop_init()

Hi, Huacai

On 2022/10/13 23:47, Huacai Chen wrote:
> Hi, Yingliang,
>
> Thank you for your patches, but could you please merge the two trivial
> patches to a single one?
It's OK to merge them into a single one in v2.
But the two patches solved different things, they are not relative.

Thanks,
Yang
>
> Huacai
>
> On Thu, Oct 13, 2022 at 9:12 PM Yang Yingliang <[email protected]> wrote:
>> Current the return value of 'sub_driver->init' is not checked,
>> if sparse_keymap_setup() called in the init function fails,
>> 'generic_inputdev' is freed, then it willl lead a UAF when
>> using it in generic_acpi_laptop_init(). Fix it by checking
>> return value. Set generic_inputdev to NULL after free to avoid
>> double free it.
>>
>> Fixes: 6246ed09111f ("LoongArch: Add ACPI-based generic laptop driver")
>> Signed-off-by: Yang Yingliang <[email protected]>
>> ---
>> drivers/platform/loongarch/loongson-laptop.c | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/platform/loongarch/loongson-laptop.c b/drivers/platform/loongarch/loongson-laptop.c
>> index f0166ad5d2c2..a665fd1042ac 100644
>> --- a/drivers/platform/loongarch/loongson-laptop.c
>> +++ b/drivers/platform/loongarch/loongson-laptop.c
>> @@ -448,6 +448,7 @@ static int __init event_init(struct generic_sub_driver *sub_driver)
>> if (ret < 0) {
>> pr_err("Failed to setup input device keymap\n");
>> input_free_device(generic_inputdev);
>> + generic_inputdev = NULL;
>>
>> return ret;
>> }
>> @@ -502,8 +503,11 @@ static int __init generic_subdriver_init(struct generic_sub_driver *sub_driver)
>> if (ret)
>> return -EINVAL;
>>
>> - if (sub_driver->init)
>> - sub_driver->init(sub_driver);
>> + if (sub_driver->init) {
>> + ret = sub_driver->init(sub_driver);
>> + if (ret)
>> + goto err_out;
>> + }
>>
>> if (sub_driver->notify) {
>> ret = setup_acpi_notify(sub_driver);
>> --
>> 2.25.1
>>
> .

2022-10-14 02:24:54

by Huacai Chen

Subject: Re: [PATCH] platform/loongarch: laptop: fix possible UAF in generic_acpi_laptop_init()

On Fri, Oct 14, 2022 at 9:23 AM Yang Yingliang <[email protected]> wrote:
>
> Hi, Huacai
>
> On 2022/10/13 23:47, Huacai Chen wrote:
> > Hi, Yingliang,
> >
> > Thank you for your patches, but could you please merge the two trivial
> > patches to a single one?
> It's OK to merge them into a single one in v2.
> But the two patches solved different things, they are not relative.
You can title it with "Simplify and improve generic_subdriver_init()" or so.

Huacai
>
> Thanks,
> Yang
> >
> > Huacai
> >
> > On Thu, Oct 13, 2022 at 9:12 PM Yang Yingliang <[email protected]> wrote:
> >> Current the return value of 'sub_driver->init' is not checked,
> >> if sparse_keymap_setup() called in the init function fails,
> >> 'generic_inputdev' is freed, then it willl lead a UAF when
> >> using it in generic_acpi_laptop_init(). Fix it by checking
> >> return value. Set generic_inputdev to NULL after free to avoid
> >> double free it.
> >>
> >> Fixes: 6246ed09111f ("LoongArch: Add ACPI-based generic laptop driver")
> >> Signed-off-by: Yang Yingliang <[email protected]>
> >> ---
> >> drivers/platform/loongarch/loongson-laptop.c | 8 ++++++--
> >> 1 file changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/platform/loongarch/loongson-laptop.c b/drivers/platform/loongarch/loongson-laptop.c
> >> index f0166ad5d2c2..a665fd1042ac 100644
> >> --- a/drivers/platform/loongarch/loongson-laptop.c
> >> +++ b/drivers/platform/loongarch/loongson-laptop.c
> >> @@ -448,6 +448,7 @@ static int __init event_init(struct generic_sub_driver *sub_driver)
> >> if (ret < 0) {
> >> pr_err("Failed to setup input device keymap\n");
> >> input_free_device(generic_inputdev);
> >> + generic_inputdev = NULL;
> >>
> >> return ret;
> >> }
> >> @@ -502,8 +503,11 @@ static int __init generic_subdriver_init(struct generic_sub_driver *sub_driver)
> >> if (ret)
> >> return -EINVAL;
> >>
> >> - if (sub_driver->init)
> >> - sub_driver->init(sub_driver);
> >> + if (sub_driver->init) {
> >> + ret = sub_driver->init(sub_driver);
> >> + if (ret)
> >> + goto err_out;
> >> + }
> >>
> >> if (sub_driver->notify) {
> >> ret = setup_acpi_notify(sub_driver);
> >> --
> >> 2.25.1
> >>
> > .