In acpi_table_initrd_override(), the return value of acpi_os_map_memory()
is assigned to table and there is a dereference of it after that.
acpi_os_map_memory() will return NULL on failure, which may lead to NULL
pointer dereference.
Fix this bug by adding a NULL check of table.
This bug was found by a static analyzer.
Builds with 'make allyesconfig' show no new warnings,
and our static analyzer no longer warns about this code.
Fixes: 5ae74f2cc2f1 ("ACPI / tables: Move table override mechanisms to tables.c")
Signed-off-by: Zhou Qingyang <[email protected]>
---
The analysis employs differential checking to identify inconsistent
security operations (e.g., checks or kfrees) between two code paths
and confirms that the inconsistent operations are not recovered in the
current function or the callers, so they constitute bugs.
Note that, as a bug found by static analysis, it can be a false
positive or hard to trigger. Multiple researchers have cross-reviewed
the bug.
drivers/acpi/tables.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index 0741a4933f62..8b10c192ed32 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -696,6 +696,10 @@ acpi_table_initrd_override(struct acpi_table_header *existing_table,
while (table_offset + ACPI_HEADER_SIZE <= all_tables_size) {
table = acpi_os_map_memory(acpi_tables_addr + table_offset,
ACPI_HEADER_SIZE);
+ if (!table) {
+ return AE_NO_MEMORY;
+ }
+
if (table_offset + table->length > all_tables_size) {
acpi_os_unmap_memory(table, ACPI_HEADER_SIZE);
WARN_ON(1);
--
2.25.1
Hi Zhou,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on rafael-pm/linux-next]
[also build test ERROR on linux/master linus/master v5.17-rc1 next-20220124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Zhou-Qingyang/ACPI-tables-Fix-a-NULL-pointer-dereference-in-acpi_table_initrd_override/20220125-004517
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: x86_64-randconfig-r011-20220124 (https://download.01.org/0day-ci/archive/20220125/[email protected]/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 2e58a18910867ba6795066e044293e6daf89edf5)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/924d4e212d2041e2af120ade3599fdc00e0b12fd
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Zhou-Qingyang/ACPI-tables-Fix-a-NULL-pointer-dereference-in-acpi_table_initrd_override/20220125-004517
git checkout 924d4e212d2041e2af120ade3599fdc00e0b12fd
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> drivers/acpi/tables.c:710:4: error: void function 'acpi_table_initrd_scan' should not return a value [-Wreturn-type]
return AE_NO_MEMORY;
^ ~~~~~~~~~~~~
1 error generated.
vim +/acpi_table_initrd_scan +710 drivers/acpi/tables.c
695
696 static void __init acpi_table_initrd_scan(void)
697 {
698 int table_offset = 0;
699 int table_index = 0;
700 u32 table_length;
701 struct acpi_table_header *table;
702
703 if (!acpi_tables_addr)
704 return;
705
706 while (table_offset + ACPI_HEADER_SIZE <= all_tables_size) {
707 table = acpi_os_map_memory(acpi_tables_addr + table_offset,
708 ACPI_HEADER_SIZE);
709 if (!table) {
> 710 return AE_NO_MEMORY;
711 }
712
713 if (table_offset + table->length > all_tables_size) {
714 acpi_os_unmap_memory(table, ACPI_HEADER_SIZE);
715 WARN_ON(1);
716 return;
717 }
718
719 table_length = table->length;
720
721 /* Skip RSDT/XSDT which should only be used for override */
722 if (ACPI_COMPARE_NAMESEG(table->signature, ACPI_SIG_RSDT) ||
723 ACPI_COMPARE_NAMESEG(table->signature, ACPI_SIG_XSDT)) {
724 acpi_os_unmap_memory(table, ACPI_HEADER_SIZE);
725 goto next_table;
726 }
727 /*
728 * Mark the table to avoid being used in
729 * acpi_table_initrd_override(). Though this is not possible
730 * because override is disabled in acpi_install_physical_table().
731 */
732 if (test_and_set_bit(table_index, acpi_initrd_installed)) {
733 acpi_os_unmap_memory(table, ACPI_HEADER_SIZE);
734 goto next_table;
735 }
736
737 pr_info("Table Upgrade: install [%4.4s-%6.6s-%8.8s]\n",
738 table->signature, table->oem_id,
739 table->oem_table_id);
740 acpi_os_unmap_memory(table, ACPI_HEADER_SIZE);
741 acpi_install_physical_table(acpi_tables_addr + table_offset);
742 next_table:
743 table_offset += table_length;
744 table_index++;
745 }
746 }
747 #else
748 static acpi_status
749 acpi_table_initrd_override(struct acpi_table_header *existing_table,
750 acpi_physical_address *address,
751 u32 *table_length)
752 {
753 *table_length = 0;
754 *address = 0;
755 return AE_OK;
756 }
757
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On Tue, Jan 25, 2022 at 12:42:51AM +0800, Zhou Qingyang wrote:
> In acpi_table_initrd_override(), the return value of acpi_os_map_memory()
> is assigned to table and there is a dereference of it after that.
> acpi_os_map_memory() will return NULL on failure, which may lead to NULL
> pointer dereference.
>
> Fix this bug by adding a NULL check of table.
>
> This bug was found by a static analyzer.
>
> Builds with 'make allyesconfig' show no new warnings,
> and our static analyzer no longer warns about this code.
>
> Fixes: 5ae74f2cc2f1 ("ACPI / tables: Move table override mechanisms to tables.c")
> Signed-off-by: Zhou Qingyang <[email protected]>
> ---
> The analysis employs differential checking to identify inconsistent
> security operations (e.g., checks or kfrees) between two code paths
> and confirms that the inconsistent operations are not recovered in the
> current function or the callers, so they constitute bugs.
>
> Note that, as a bug found by static analysis, it can be a false
> positive or hard to trigger. Multiple researchers have cross-reviewed
> the bug.
>
> drivers/acpi/tables.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
> index 0741a4933f62..8b10c192ed32 100644
> --- a/drivers/acpi/tables.c
> +++ b/drivers/acpi/tables.c
> @@ -696,6 +696,10 @@ acpi_table_initrd_override(struct acpi_table_header *existing_table,
> while (table_offset + ACPI_HEADER_SIZE <= all_tables_size) {
> table = acpi_os_map_memory(acpi_tables_addr + table_offset,
> ACPI_HEADER_SIZE);
> + if (!table) {
> + return AE_NO_MEMORY;
> + }
> +
> if (table_offset + table->length > all_tables_size) {
> acpi_os_unmap_memory(table, ACPI_HEADER_SIZE);
> WARN_ON(1);
> --
> 2.25.1
>
As stated before, umn.edu is still not allowed to contribute to the
Linux kernel. Please work with your administration to resolve this
issue.