| Message ID | 20210427014134.3568-1-lyl2019@mail.ustc.edu.cn |
|---|---|
| State | New |
| Headers | show |
| Series | ACPICA:dbnames: Fix a error free in acpi_db_walk_for_fields | expand |
> -----Original Message----- > From: Lv Yunlong <lyl2019@mail.ustc.edu.cn> > Sent: Monday, April 26, 2021 6:42 PM > To: Moore, Robert <robert.moore@intel.com>; Kaneda, Erik > <erik.kaneda@intel.com>; Wysocki, Rafael J <rafael.j.wysocki@intel.com>; > lenb@kernel.org > Cc: linux-acpi@vger.kernel.org; devel@acpica.org; linux- > kernel@vger.kernel.org; Lv Yunlong <lyl2019@mail.ustc.edu.cn> > Subject: [PATCH] ACPICA:dbnames: Fix a error free in > acpi_db_walk_for_fields > > In acpi_db_walk_for_fields, buffer.pointer is freed in the first > time via ACPI_FREE() after acpi_os_printf("%s ", (char *)buffer.pointer). > But later, buffer.pointer is assigned to ret_value, and the freed > pointer is dereferenced by ret_value, which is use after free. > > In addition, buffer.pointer is freed by ACPI_FREE() again after > acpi_os_printf("}\n"), which is a double free. > > My patch removes the first ACPI_FREE() to avoid the uaf and double > free bugs. > I'll take a look Thanks > Fixes: 5fd033288a866 ("ACPICA: debugger: add command to dump all fields > of particular subtype") > Signed-off-by: Lv Yunlong <lyl2019@mail.ustc.edu.cn> > --- > drivers/acpi/acpica/dbnames.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/acpi/acpica/dbnames.c b/drivers/acpi/acpica/dbnames.c > index 3615e1a6efd8..dabd76df15ec 100644 > --- a/drivers/acpi/acpica/dbnames.c > +++ b/drivers/acpi/acpica/dbnames.c > @@ -547,7 +547,6 @@ acpi_db_walk_for_fields(acpi_handle obj_handle, > } > > acpi_os_printf("%s ", (char *)buffer.pointer); > - ACPI_FREE(buffer.pointer); > > buffer.length = ACPI_ALLOCATE_LOCAL_BUFFER; > acpi_evaluate_object(obj_handle, NULL, NULL, &buffer); > -- > 2.25.1 >
> -----Original Message----- > From: Kaneda, Erik <erik.kaneda@intel.com> > Sent: Thursday, April 29, 2021 11:01 AM > To: Lv Yunlong <lyl2019@mail.ustc.edu.cn>; Moore, Robert > <robert.moore@intel.com>; Wysocki, Rafael J <rafael.j.wysocki@intel.com>; > lenb@kernel.org > Cc: linux-acpi@vger.kernel.org; devel@acpica.org; linux- > kernel@vger.kernel.org > Subject: RE: [PATCH] ACPICA:dbnames: Fix a error free in > acpi_db_walk_for_fields > > > > > -----Original Message----- > > From: Lv Yunlong <lyl2019@mail.ustc.edu.cn> > > Sent: Monday, April 26, 2021 6:42 PM > > To: Moore, Robert <robert.moore@intel.com>; Kaneda, Erik > > <erik.kaneda@intel.com>; Wysocki, Rafael J <rafael.j.wysocki@intel.com>; > > lenb@kernel.org > > Cc: linux-acpi@vger.kernel.org; devel@acpica.org; linux- > > kernel@vger.kernel.org; Lv Yunlong <lyl2019@mail.ustc.edu.cn> > > Subject: [PATCH] ACPICA:dbnames: Fix a error free in > > acpi_db_walk_for_fields > > > > In acpi_db_walk_for_fields, buffer.pointer is freed in the first > > time via ACPI_FREE() after acpi_os_printf("%s ", (char *)buffer.pointer). > > But later, buffer.pointer is assigned to ret_value, and the freed > > pointer is dereferenced by ret_value, which is use after free. > > > > In addition, buffer.pointer is freed by ACPI_FREE() again after > > acpi_os_printf("}\n"), which is a double free. > > > > My patch removes the first ACPI_FREE() to avoid the uaf and double > > free bugs. > > > > I'll take a look > > Thanks > > > Fixes: 5fd033288a866 ("ACPICA: debugger: add command to dump all fields > > of particular subtype") > > Signed-off-by: Lv Yunlong <lyl2019@mail.ustc.edu.cn> > > --- > > drivers/acpi/acpica/dbnames.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/drivers/acpi/acpica/dbnames.c b/drivers/acpi/acpica/dbnames.c > > index 3615e1a6efd8..dabd76df15ec 100644 > > --- a/drivers/acpi/acpica/dbnames.c > > +++ b/drivers/acpi/acpica/dbnames.c > > @@ -547,7 +547,6 @@ acpi_db_walk_for_fields(acpi_handle obj_handle, > > } > > > > acpi_os_printf("%s ", (char *)buffer.pointer); > > - ACPI_FREE(buffer.pointer); This is freeing the pointer allocated by acpi_ns_handle_to_pathname > > > > buffer.length = ACPI_ALLOCATE_LOCAL_BUFFER; > > acpi_evaluate_object(obj_handle, NULL, NULL, &buffer); This call to acpi_evaluate_object will allocate an object in Buffer.Pointer object so the dereference of buffer->pointer is not a use after free > > -- > > 2.25.1 > >
diff --git a/drivers/acpi/acpica/dbnames.c b/drivers/acpi/acpica/dbnames.c index 3615e1a6efd8..dabd76df15ec 100644 --- a/drivers/acpi/acpica/dbnames.c +++ b/drivers/acpi/acpica/dbnames.c @@ -547,7 +547,6 @@ acpi_db_walk_for_fields(acpi_handle obj_handle, } acpi_os_printf("%s ", (char *)buffer.pointer); - ACPI_FREE(buffer.pointer); buffer.length = ACPI_ALLOCATE_LOCAL_BUFFER; acpi_evaluate_object(obj_handle, NULL, NULL, &buffer);
In acpi_db_walk_for_fields, buffer.pointer is freed in the first time via ACPI_FREE() after acpi_os_printf("%s ", (char *)buffer.pointer). But later, buffer.pointer is assigned to ret_value, and the freed pointer is dereferenced by ret_value, which is use after free. In addition, buffer.pointer is freed by ACPI_FREE() again after acpi_os_printf("}\n"), which is a double free. My patch removes the first ACPI_FREE() to avoid the uaf and double free bugs. Fixes: 5fd033288a866 ("ACPICA: debugger: add command to dump all fields of particular subtype") Signed-off-by: Lv Yunlong <lyl2019@mail.ustc.edu.cn> --- drivers/acpi/acpica/dbnames.c | 1 - 1 file changed, 1 deletion(-)