[v1,1/2] ACPI: property: Allow _DSD buffer data only for byte accessors

Message ID 20231002134630.2601294-1-andriy.shevchenko@linux.intel.com
State New
Headers
Series [v1,1/2] ACPI: property: Allow _DSD buffer data only for byte accessors |

Commit Message

Andy Shevchenko Oct. 2, 2023, 1:46 p.m. UTC
  In accordance with ACPI specificication and _DSD data buffer
representation the data there is an array of bytes. Hence,
accessing it with something longer will create a sparse data
which is against of how device property APIs work in general
and also not defined in the ACPI specification (see [1]).
Fix the code to emit an error if non-byte accessor is used to
retrieve _DSD buffer data.

Fixes: 369af6bf2c28 ("ACPI: property: Read buffer properties as integers")
Link: https://uefi.org/specs/ACPI/6.5/19_ASL_Reference.html#buffer-declare-buffer-object [1]
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/acpi/property.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)
  

Comments

Rafael J. Wysocki Oct. 3, 2023, 1:59 p.m. UTC | #1
On Mon, Oct 2, 2023 at 3:46 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> In accordance with ACPI specificication and _DSD data buffer
> representation the data there is an array of bytes. Hence,
> accessing it with something longer will create a sparse data
> which is against of how device property APIs work in general
> and also not defined in the ACPI specification (see [1]).
> Fix the code to emit an error if non-byte accessor is used to
> retrieve _DSD buffer data.
>
> Fixes: 369af6bf2c28 ("ACPI: property: Read buffer properties as integers")
> Link: https://uefi.org/specs/ACPI/6.5/19_ASL_Reference.html#buffer-declare-buffer-object [1]
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied along with the [2/2] as 6.7 material.

> ---
>  drivers/acpi/property.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index 413e4fcadcaf..06550d8c619d 100644
> --- a/drivers/acpi/property.c
> +++ b/drivers/acpi/property.c
> @@ -1102,25 +1102,25 @@ static int acpi_data_prop_read(const struct acpi_device_data *data,
>         switch (proptype) {
>         case DEV_PROP_STRING:
>                 break;
> -       case DEV_PROP_U8 ... DEV_PROP_U64:
> +       default:
>                 if (obj->type == ACPI_TYPE_BUFFER) {
>                         if (nval > obj->buffer.length)
>                                 return -EOVERFLOW;
> -                       break;
> +               } else {
> +                       if (nval > obj->package.count)
> +                               return -EOVERFLOW;
>                 }
> -               fallthrough;
> -       default:
> -               if (nval > obj->package.count)
> -                       return -EOVERFLOW;
>                 break;
>         }
>         if (nval == 0)
>                 return -EINVAL;
>
> -       if (obj->type != ACPI_TYPE_BUFFER)
> -               items = obj->package.elements;
> -       else
> +       if (obj->type == ACPI_TYPE_BUFFER) {
> +               if (proptype != DEV_PROP_U8)
> +                       return -EPROTO;
>                 items = obj;
> +       } else
> +               items = obj->package.elements;

The braces that are missing here (as per the coding style) were added
while applying the patch.

>
>         switch (proptype) {
>         case DEV_PROP_U8:
> --

Thanks!
  
Andy Shevchenko Oct. 3, 2023, 2:05 p.m. UTC | #2
On Tue, Oct 03, 2023 at 03:59:26PM +0200, Rafael J. Wysocki wrote:
> On Mon, Oct 2, 2023 at 3:46 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:

...

> > +       if (obj->type == ACPI_TYPE_BUFFER) {
> > +               if (proptype != DEV_PROP_U8)
> > +                       return -EPROTO;
> >                 items = obj;
> > +       } else
> > +               items = obj->package.elements;
> 
> The braces that are missing here (as per the coding style) were added
> while applying the patch.

Thank you!
  

Patch

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 413e4fcadcaf..06550d8c619d 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -1102,25 +1102,25 @@  static int acpi_data_prop_read(const struct acpi_device_data *data,
 	switch (proptype) {
 	case DEV_PROP_STRING:
 		break;
-	case DEV_PROP_U8 ... DEV_PROP_U64:
+	default:
 		if (obj->type == ACPI_TYPE_BUFFER) {
 			if (nval > obj->buffer.length)
 				return -EOVERFLOW;
-			break;
+		} else {
+			if (nval > obj->package.count)
+				return -EOVERFLOW;
 		}
-		fallthrough;
-	default:
-		if (nval > obj->package.count)
-			return -EOVERFLOW;
 		break;
 	}
 	if (nval == 0)
 		return -EINVAL;
 
-	if (obj->type != ACPI_TYPE_BUFFER)
-		items = obj->package.elements;
-	else
+	if (obj->type == ACPI_TYPE_BUFFER) {
+		if (proptype != DEV_PROP_U8)
+			return -EPROTO;
 		items = obj;
+	} else
+		items = obj->package.elements;
 
 	switch (proptype) {
 	case DEV_PROP_U8: