[PATCH] ACPI / tables: Fix a NULL pointer dereference in acpi_table_initrd_override()

[PATCH] ACPI / tables: Fix a NULL pointer dereference in acpi_table_initrd_override()

[Zhou Qingyang]

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 <zhou1615@umn.edu>
---
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

Re: [PATCH] ACPI / tables: Fix a NULL pointer dereference in acpi_table_initrd_override()

[kernel test robot]

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/202201251123.ScbhjE9a-lkp@intel.com/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 <lkp@intel.com>

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/kbuild-all@lists.01.org

Re: [PATCH] ACPI / tables: Fix a NULL pointer dereference in acpi_table_initrd_override()

[Greg KH]

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 <zhou1615@umn.edu>
> ---
> 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.