[PATCH] acpi: fix acpi_get_table() leak / acpi-sysfs denial of service

[PATCH] acpi: fix acpi_get_table() leak / acpi-sysfs denial of service

[Dan Williams]

Reading an ACPI table through the /sys/firmware/acpi/tables interface
more than 65,536 times leads to the following log message:

 ACPI Error: Table ffff88033595eaa8, Validation count is zero after increment
  (20170119/tbutils-423)

...and the table being unavailable until the next reboot. Add the
missing acpi_put_table() so the table ->validation_count is decremented
after each read.

Cc: <stable@vger.kernel.org>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Rafael Wysocki <rafael.j.wysocki@intel.com>
Cc: Kristin Jacque <kristin.jacque@intel.com>
Cc: Tiffany Kasanicky <tiffany.j.kasanicky@intel.com>
Cc: Ryon Jensen <ryon.jensen@intel.com>
Reported-by: Anush Seetharaman <anush.seetharaman@intel.com>
Fixes: 1c8fce27e275 ("ACPI: introduce drivers/acpi/sysfs.c")
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/acpi/sysfs.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
index cf05ae973381..531ddabf0390 100644
--- a/drivers/acpi/sysfs.c
+++ b/drivers/acpi/sysfs.c
@@ -333,14 +333,17 @@ static ssize_t acpi_table_show(struct file *filp, struct kobject *kobj,
 	    container_of(bin_attr, struct acpi_table_attr, attr);
 	struct acpi_table_header *table_header = NULL;
 	acpi_status status;
+	ssize_t rc;
 
 	status = acpi_get_table(table_attr->name, table_attr->instance,
 				&table_header);
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
-	return memory_read_from_buffer(buf, count, &offset,
-				       table_header, table_header->length);
+	rc = memory_read_from_buffer(buf, count, &offset, table_header,
+			table_header->length);
+	acpi_put_table(table);
+	return rc;
 }
 
 static int acpi_table_attr_init(struct kobject *tables_obj,

RE: [PATCH] acpi: fix acpi_get_table() leak / acpi-sysfs denial of service

[Zheng, Lv]

Hi,

> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Dan
> Williams
> Subject: [PATCH] acpi: fix acpi_get_table() leak / acpi-sysfs denial of service
> 
> Reading an ACPI table through the /sys/firmware/acpi/tables interface
> more than 65,536 times leads to the following log message:
> 
>  ACPI Error: Table ffff88033595eaa8, Validation count is zero after increment
>   (20170119/tbutils-423)
> 
> ...and the table being unavailable until the next reboot. Add the
> missing acpi_put_table() so the table ->validation_count is decremented
> after each read.

For now, should just return AE_OK instead of AE_LIMIT in acpi_tb_get_table().
Otherwise this fix will not be sufficient to fix all possible problem
Unless we can add acpi_put_table() for all acpi_get_table() invocation.
Let me send a patch for you to try.

Thanks and best regards
Lv

> 
> Cc: <stable@vger.kernel.org>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Rafael Wysocki <rafael.j.wysocki@intel.com>
> Cc: Kristin Jacque <kristin.jacque@intel.com>
> Cc: Tiffany Kasanicky <tiffany.j.kasanicky@intel.com>
> Cc: Ryon Jensen <ryon.jensen@intel.com>
> Reported-by: Anush Seetharaman <anush.seetharaman@intel.com>
> Fixes: 1c8fce27e275 ("ACPI: introduce drivers/acpi/sysfs.c")
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/acpi/sysfs.c |    7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
> index cf05ae973381..531ddabf0390 100644
> --- a/drivers/acpi/sysfs.c
> +++ b/drivers/acpi/sysfs.c
> @@ -333,14 +333,17 @@ static ssize_t acpi_table_show(struct file *filp, struct kobject *kobj,
>  	    container_of(bin_attr, struct acpi_table_attr, attr);
>  	struct acpi_table_header *table_header = NULL;
>  	acpi_status status;
> +	ssize_t rc;
> 
>  	status = acpi_get_table(table_attr->name, table_attr->instance,
>  				&table_header);
>  	if (ACPI_FAILURE(status))
>  		return -ENODEV;
> 
> -	return memory_read_from_buffer(buf, count, &offset,
> -				       table_header, table_header->length);
> +	rc = memory_read_from_buffer(buf, count, &offset, table_header,
> +			table_header->length);
> +	acpi_put_table(table);
> +	return rc;
>  }
> 
>  static int acpi_table_attr_init(struct kobject *tables_obj,
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] acpi: fix acpi_get_table() leak / acpi-sysfs denial of service

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2067 bytes --]

Hi Dan,

[auto build test ERROR on pm/linux-next]
[also build test ERROR on v4.11-rc8 next-20170426]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Dan-Williams/acpi-fix-acpi_get_table-leak-acpi-sysfs-denial-of-service/20170427-005809
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: x86_64-kexec (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers//acpi/sysfs.c: In function 'acpi_table_show':
>> drivers//acpi/sysfs.c:345:17: error: 'table' undeclared (first use in this function)
     acpi_put_table(table);
                    ^~~~~
   drivers//acpi/sysfs.c:345:17: note: each undeclared identifier is reported only once for each function it appears in

vim +/table +345 drivers//acpi/sysfs.c

   329				       struct bin_attribute *bin_attr, char *buf,
   330				       loff_t offset, size_t count)
   331	{
   332		struct acpi_table_attr *table_attr =
   333		    container_of(bin_attr, struct acpi_table_attr, attr);
   334		struct acpi_table_header *table_header = NULL;
   335		acpi_status status;
   336		ssize_t rc;
   337	
   338		status = acpi_get_table(table_attr->name, table_attr->instance,
   339					&table_header);
   340		if (ACPI_FAILURE(status))
   341			return -ENODEV;
   342	
   343		rc = memory_read_from_buffer(buf, count, &offset, table_header,
   344				table_header->length);
 > 345		acpi_put_table(table);
   346		return rc;
   347	}
   348	
   349	static int acpi_table_attr_init(struct kobject *tables_obj,
   350					struct acpi_table_attr *table_attr,
   351					struct acpi_table_header *table_header)
   352	{
   353		struct acpi_table_header *header = NULL;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25155 bytes --]