| Message ID | 20220828205416.1957305-1-floridsleeves@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1] drivers/acpi/processor_idle: check the return value of acpi_fetch_acpi_dev() | expand |
On Mon, Aug 29, 2022 at 6:56 AM Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Sun, Aug 28, 2022 at 10:54 PM Li Zhong <floridsleeves@gmail.com> wrote: > > > > The return value of acpi_fetch_acpi_dev() could be NULL, which will > > cause null pointer dereference if used in acpi_device_hid(). > > That's true. > > > Signed-off-by: Li Zhong <floridsleeves@gmail.com> > > --- > > drivers/acpi/processor_idle.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c > > index 16a1663d02d4..519f8f741da3 100644 > > --- a/drivers/acpi/processor_idle.c > > +++ b/drivers/acpi/processor_idle.c > > @@ -1117,6 +1117,8 @@ static int acpi_processor_get_lpi_info(struct acpi_processor *pr) > > status = acpi_get_parent(handle, &pr_ahandle); > > while (ACPI_SUCCESS(status)) { > > d = acpi_fetch_acpi_dev(pr_ahandle); > > + if (!d) > > + break; > > But shouldn't this be continue? > I think here is break instead of continue because if we use continue, variable status will not change. Then the while condition will stay true and loop forever. > > handle = pr_ahandle; > > > > if (strcmp(acpi_device_hid(d), ACPI_PROCESSOR_CONTAINER_HID)) > > -- > > 2.25.1 > >
On Sun, Aug 28, 2022 at 10:54 PM Li Zhong <floridsleeves@gmail.com> wrote: > > The return value of acpi_fetch_acpi_dev() could be NULL, which will > cause null pointer dereference if used in acpi_device_hid(). > > Signed-off-by: Li Zhong <floridsleeves@gmail.com> > --- > drivers/acpi/processor_idle.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c > index 16a1663d02d4..519f8f741da3 100644 > --- a/drivers/acpi/processor_idle.c > +++ b/drivers/acpi/processor_idle.c > @@ -1117,6 +1117,8 @@ static int acpi_processor_get_lpi_info(struct acpi_processor *pr) > status = acpi_get_parent(handle, &pr_ahandle); > while (ACPI_SUCCESS(status)) { > d = acpi_fetch_acpi_dev(pr_ahandle); > + if (!d) > + break; > handle = pr_ahandle; > > if (strcmp(acpi_device_hid(d), ACPI_PROCESSOR_CONTAINER_HID)) > -- Applied (with some edits in the subject and changelog) as 6.1 material, thanks!
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index 16a1663d02d4..519f8f741da3 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -1117,6 +1117,8 @@ static int acpi_processor_get_lpi_info(struct acpi_processor *pr) status = acpi_get_parent(handle, &pr_ahandle); while (ACPI_SUCCESS(status)) { d = acpi_fetch_acpi_dev(pr_ahandle); + if (!d) + break; handle = pr_ahandle; if (strcmp(acpi_device_hid(d), ACPI_PROCESSOR_CONTAINER_HID))
The return value of acpi_fetch_acpi_dev() could be NULL, which will cause null pointer dereference if used in acpi_device_hid(). Signed-off-by: Li Zhong <floridsleeves@gmail.com> --- drivers/acpi/processor_idle.c | 2 ++ 1 file changed, 2 insertions(+)