* Re: [PATCH v4 3/3] lib/firmware_table: Provide buffer length argument to cdat_table_parse()
[not found] <20240216155844.406996-4-rrichter@amd.com>
@ 2024-02-17 10:43 ` kernel test robot
2024-02-17 21:39 ` [PATCH v5] " Robert Richter
2024-02-18 12:58 ` [PATCH v4 3/3] " kernel test robot
1 sibling, 1 reply; 4+ messages in thread
From: kernel test robot @ 2024-02-17 10:43 UTC (permalink / raw
To: Robert Richter, Alison Schofield, Vishal Verma, Ira Weiny,
Dan Williams, Jonathan Cameron, Dave Jiang, Davidlohr Bueso,
Rafael J. Wysocki, Andrew Morton
Cc: oe-kbuild-all, Linux Memory Management List, linux-cxl,
linux-kernel, Len Brown, Robert Richter, linux-acpi
Hi Robert,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 6be99530c92c6b8ff7a01903edc42393575ad63b]
url: https://github.com/intel-lab-lkp/linux/commits/Robert-Richter/cxl-pci-Rename-DOE-mailbox-handle-to-doe_mb/20240217-000206
base: 6be99530c92c6b8ff7a01903edc42393575ad63b
patch link: https://lore.kernel.org/r/20240216155844.406996-4-rrichter%40amd.com
patch subject: [PATCH v4 3/3] lib/firmware_table: Provide buffer length argument to cdat_table_parse()
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20240217/202402171817.i0WShbft-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240217/202402171817.i0WShbft-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402171817.i0WShbft-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from include/linux/device.h:15,
from drivers/cxl/core/pci.c:5:
drivers/cxl/core/pci.c: In function 'read_cdat_data':
>> drivers/cxl/core/pci.c:672:31: warning: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
672 | dev_warn(dev, "Malformed CDAT table length (%lu:%lu), discarding trailing data\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:146:61: note: in expansion of macro 'dev_fmt'
146 | dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/cxl/core/pci.c:672:17: note: in expansion of macro 'dev_warn'
672 | dev_warn(dev, "Malformed CDAT table length (%lu:%lu), discarding trailing data\n",
| ^~~~~~~~
drivers/cxl/core/pci.c:672:63: note: format string is defined here
672 | dev_warn(dev, "Malformed CDAT table length (%lu:%lu), discarding trailing data\n",
| ~~^
| |
| long unsigned int
| %u
drivers/cxl/core/pci.c:672:31: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
672 | dev_warn(dev, "Malformed CDAT table length (%lu:%lu), discarding trailing data\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:146:61: note: in expansion of macro 'dev_fmt'
146 | dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/cxl/core/pci.c:672:17: note: in expansion of macro 'dev_warn'
672 | dev_warn(dev, "Malformed CDAT table length (%lu:%lu), discarding trailing data\n",
| ^~~~~~~~
drivers/cxl/core/pci.c:672:67: note: format string is defined here
672 | dev_warn(dev, "Malformed CDAT table length (%lu:%lu), discarding trailing data\n",
| ~~^
| |
| long unsigned int
| %u
during RTL pass: mach
drivers/cxl/core/pci.c: In function 'match_add_dports':
drivers/cxl/core/pci.c:68:1: internal compiler error: in arc_ifcvt, at config/arc/arc.cc:9703
68 | }
| ^
0x5b78c1 arc_ifcvt
/tmp/build-crosstools-gcc-13.2.0-binutils-2.41/gcc/gcc-13.2.0/gcc/config/arc/arc.cc:9703
0xe431b4 arc_reorg
/tmp/build-crosstools-gcc-13.2.0-binutils-2.41/gcc/gcc-13.2.0/gcc/config/arc/arc.cc:8552
0xaed299 execute
/tmp/build-crosstools-gcc-13.2.0-binutils-2.41/gcc/gcc-13.2.0/gcc/reorg.cc:3927
Please submit a full bug report, with preprocessed source (by using -freport-bug).
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
vim +672 drivers/cxl/core/pci.c
611
612 /**
613 * read_cdat_data - Read the CDAT data on this port
614 * @port: Port to read data from
615 *
616 * This call will sleep waiting for responses from the DOE mailbox.
617 */
618 void read_cdat_data(struct cxl_port *port)
619 {
620 struct device *uport = port->uport_dev;
621 struct device *dev = &port->dev;
622 struct pci_doe_mb *doe_mb;
623 struct pci_dev *pdev = NULL;
624 struct cxl_memdev *cxlmd;
625 struct cdat_doe_rsp *buf;
626 size_t table_length, length;
627 int rc;
628
629 if (is_cxl_memdev(uport)) {
630 struct device *host;
631
632 cxlmd = to_cxl_memdev(uport);
633 host = cxlmd->dev.parent;
634 if (dev_is_pci(host))
635 pdev = to_pci_dev(host);
636 } else if (dev_is_pci(uport)) {
637 pdev = to_pci_dev(uport);
638 }
639
640 if (!pdev)
641 return;
642
643 doe_mb = pci_find_doe_mailbox(pdev, PCI_DVSEC_VENDOR_ID_CXL,
644 CXL_DOE_PROTOCOL_TABLE_ACCESS);
645 if (!doe_mb) {
646 dev_dbg(dev, "No CDAT mailbox\n");
647 return;
648 }
649
650 port->cdat_available = true;
651
652 if (cxl_cdat_get_length(dev, doe_mb, &length)) {
653 dev_dbg(dev, "No CDAT length\n");
654 return;
655 }
656
657 /*
658 * The begin of the CDAT buffer needs space for additional 4
659 * bytes for the DOE header. Table data starts afterwards.
660 */
661 buf = devm_kzalloc(dev, sizeof(*buf) + length, GFP_KERNEL);
662 if (!buf)
663 goto err;
664
665 table_length = length;
666
667 rc = cxl_cdat_read_table(dev, doe_mb, buf, &length);
668 if (rc)
669 goto err;
670
671 if (table_length != length)
> 672 dev_warn(dev, "Malformed CDAT table length (%lu:%lu), discarding trailing data\n",
673 table_length, length);
674
675 if (cdat_checksum(buf->data, length))
676 goto err;
677
678 port->cdat.table = buf->data;
679 port->cdat.length = length;
680
681 return;
682 err:
683 /* Don't leave table data allocated on error */
684 devm_kfree(dev, buf);
685 dev_err(dev, "Failed to read/validate CDAT.\n");
686 }
687 EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL);
688
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v5] lib/firmware_table: Provide buffer length argument to cdat_table_parse()
2024-02-17 10:43 ` [PATCH v4 3/3] lib/firmware_table: Provide buffer length argument to cdat_table_parse() kernel test robot
@ 2024-02-17 21:39 ` Robert Richter
2024-02-19 12:53 ` Jonathan Cameron
0 siblings, 1 reply; 4+ messages in thread
From: Robert Richter @ 2024-02-17 21:39 UTC (permalink / raw
To: Dan Williams
Cc: kernel test robot, Alison Schofield, Vishal Verma, Ira Weiny,
Jonathan Cameron, Dave Jiang, Davidlohr Bueso, Rafael J. Wysocki,
Andrew Morton, oe-kbuild-all, Linux Memory Management List,
linux-cxl, linux-kernel, Len Brown, linux-acpi
On 17.02.24 18:43:37, kernel test robot wrote:
> Hi Robert,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on 6be99530c92c6b8ff7a01903edc42393575ad63b]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Robert-Richter/cxl-pci-Rename-DOE-mailbox-handle-to-doe_mb/20240217-000206
> base: 6be99530c92c6b8ff7a01903edc42393575ad63b
> patch link: https://lore.kernel.org/r/20240216155844.406996-4-rrichter%40amd.com
> patch subject: [PATCH v4 3/3] lib/firmware_table: Provide buffer length argument to cdat_table_parse()
> config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20240217/202402171817.i0WShbft-lkp@intel.com/config)
> compiler: arceb-elf-gcc (GCC) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240217/202402171817.i0WShbft-lkp@intel.com/reproduce)
> In file included from include/linux/device.h:15,
> from drivers/cxl/core/pci.c:5:
> drivers/cxl/core/pci.c: In function 'read_cdat_data':
> >> drivers/cxl/core/pci.c:672:31: warning: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
> 672 | dev_warn(dev, "Malformed CDAT table length (%lu:%lu), discarding trailing data\n",
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Fix below, it basically uses %zu for both format strings.
-Robert
From 08685053a91e370fd1263b921aa3e8942025c4e4 Mon Sep 17 00:00:00 2001
From: Robert Richter <rrichter@amd.com>
Date: Sun, 7 Jan 2024 18:13:16 +0100
Subject: [PATCH v5] lib/firmware_table: Provide buffer length argument to
cdat_table_parse()
There exist card implementations with a CDAT table using a fixed size
buffer, but with entries filled in that do not fill the whole table
length size. Then, the last entry in the CDAT table may not mark the
end of the CDAT table buffer specified by the length field in the CDAT
header. It can be shorter with trailing unused (zero'ed) data. The
actual table length is determined while reading all CDAT entries of
the table with DOE.
If the table is greater than expected (containing zero'ed trailing
data), the CDAT parser fails with:
[ 48.691717] Malformed DSMAS table length: (24:0)
[ 48.702084] [CDAT:0x00] Invalid zero length
[ 48.711460] cxl_port endpoint1: Failed to parse CDAT: -22
In addition, a check of the table buffer length is missing to prevent
an out-of-bound access then parsing the CDAT table.
Hardening code against device returning borked table. Fix that by
providing an optional buffer length argument to
acpi_parse_entries_array() that can be used by cdat_table_parse() to
propagate the buffer size down to its users to check the buffer
length. This also prevents a possible out-of-bound access mentioned.
Add a check to warn about a malformed CDAT table length.
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Signed-off-by: Robert Richter <rrichter@amd.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Robert Richter <rrichter@amd.com>
---
drivers/acpi/tables.c | 2 +-
drivers/cxl/core/cdat.c | 6 +++---
drivers/cxl/core/pci.c | 8 +++++++-
include/linux/fw_table.h | 4 +++-
lib/fw_table.c | 15 ++++++++++-----
5 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index b07f7d091d13..b976e5fc3fbc 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -253,7 +253,7 @@ int __init_or_acpilib acpi_table_parse_entries_array(
count = acpi_parse_entries_array(id, table_size,
(union fw_table_header *)table_header,
- proc, proc_num, max_entries);
+ 0, proc, proc_num, max_entries);
acpi_put_table(table_header);
return count;
diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
index 6fe11546889f..012d8f2a7945 100644
--- a/drivers/cxl/core/cdat.c
+++ b/drivers/cxl/core/cdat.c
@@ -149,13 +149,13 @@ static int cxl_cdat_endpoint_process(struct cxl_port *port,
int rc;
rc = cdat_table_parse(ACPI_CDAT_TYPE_DSMAS, cdat_dsmas_handler,
- dsmas_xa, port->cdat.table);
+ dsmas_xa, port->cdat.table, port->cdat.length);
rc = cdat_table_parse_output(rc);
if (rc)
return rc;
rc = cdat_table_parse(ACPI_CDAT_TYPE_DSLBIS, cdat_dslbis_handler,
- dsmas_xa, port->cdat.table);
+ dsmas_xa, port->cdat.table, port->cdat.length);
return cdat_table_parse_output(rc);
}
@@ -511,7 +511,7 @@ void cxl_switch_parse_cdat(struct cxl_port *port)
return;
rc = cdat_table_parse(ACPI_CDAT_TYPE_SSLBIS, cdat_sslbis_handler,
- port, port->cdat.table);
+ port, port->cdat.table, port->cdat.length);
rc = cdat_table_parse_output(rc);
if (rc)
dev_dbg(&port->dev, "Failed to parse SSLBIS: %d\n", rc);
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 763c39456228..4e07465bf29e 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -623,7 +623,7 @@ void read_cdat_data(struct cxl_port *port)
struct pci_dev *pdev = NULL;
struct cxl_memdev *cxlmd;
struct cdat_doe_rsp *buf;
- size_t length;
+ size_t table_length, length;
int rc;
if (is_cxl_memdev(uport)) {
@@ -662,10 +662,16 @@ void read_cdat_data(struct cxl_port *port)
if (!buf)
goto err;
+ table_length = length;
+
rc = cxl_cdat_read_table(dev, doe_mb, buf, &length);
if (rc)
goto err;
+ if (table_length != length)
+ dev_warn(dev, "Malformed CDAT table length (%zu:%zu), discarding trailing data\n",
+ table_length, length);
+
if (cdat_checksum(buf->data, length))
goto err;
diff --git a/include/linux/fw_table.h b/include/linux/fw_table.h
index 95421860397a..3ff4c277296f 100644
--- a/include/linux/fw_table.h
+++ b/include/linux/fw_table.h
@@ -40,12 +40,14 @@ union acpi_subtable_headers {
int acpi_parse_entries_array(char *id, unsigned long table_size,
union fw_table_header *table_header,
+ unsigned long max_length,
struct acpi_subtable_proc *proc,
int proc_num, unsigned int max_entries);
int cdat_table_parse(enum acpi_cdat_type type,
acpi_tbl_entry_handler_arg handler_arg, void *arg,
- struct acpi_table_cdat *table_header);
+ struct acpi_table_cdat *table_header,
+ unsigned long length);
/* CXL is the only non-ACPI consumer of the FIRMWARE_TABLE library */
#if IS_ENABLED(CONFIG_ACPI) && !IS_ENABLED(CONFIG_CXL_BUS)
diff --git a/lib/fw_table.c b/lib/fw_table.c
index c3569d2ba503..16291814450e 100644
--- a/lib/fw_table.c
+++ b/lib/fw_table.c
@@ -127,6 +127,7 @@ static __init_or_fwtbl_lib int call_handler(struct acpi_subtable_proc *proc,
*
* @id: table id (for debugging purposes)
* @table_size: size of the root table
+ * @max_length: maximum size of the table (ignore if 0)
* @table_header: where does the table start?
* @proc: array of acpi_subtable_proc struct containing entry id
* and associated handler with it
@@ -148,18 +149,21 @@ static __init_or_fwtbl_lib int call_handler(struct acpi_subtable_proc *proc,
int __init_or_fwtbl_lib
acpi_parse_entries_array(char *id, unsigned long table_size,
union fw_table_header *table_header,
+ unsigned long max_length,
struct acpi_subtable_proc *proc,
int proc_num, unsigned int max_entries)
{
- unsigned long table_end, subtable_len, entry_len;
+ unsigned long table_len, table_end, subtable_len, entry_len;
struct acpi_subtable_entry entry;
enum acpi_subtable_type type;
int count = 0;
int i;
type = acpi_get_subtable_type(id);
- table_end = (unsigned long)table_header +
- acpi_table_get_length(type, table_header);
+ table_len = acpi_table_get_length(type, table_header);
+ if (max_length && max_length < table_len)
+ table_len = max_length;
+ table_end = (unsigned long)table_header + table_len;
/* Parse all entries looking for a match. */
@@ -208,7 +212,8 @@ int __init_or_fwtbl_lib
cdat_table_parse(enum acpi_cdat_type type,
acpi_tbl_entry_handler_arg handler_arg,
void *arg,
- struct acpi_table_cdat *table_header)
+ struct acpi_table_cdat *table_header,
+ unsigned long length)
{
struct acpi_subtable_proc proc = {
.id = type,
@@ -222,6 +227,6 @@ cdat_table_parse(enum acpi_cdat_type type,
return acpi_parse_entries_array(ACPI_SIG_CDAT,
sizeof(struct acpi_table_cdat),
(union fw_table_header *)table_header,
- &proc, 1, 0);
+ length, &proc, 1, 0);
}
EXPORT_SYMBOL_FWTBL_LIB(cdat_table_parse);
--
2.39.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v4 3/3] lib/firmware_table: Provide buffer length argument to cdat_table_parse()
[not found] <20240216155844.406996-4-rrichter@amd.com>
2024-02-17 10:43 ` [PATCH v4 3/3] lib/firmware_table: Provide buffer length argument to cdat_table_parse() kernel test robot
@ 2024-02-18 12:58 ` kernel test robot
1 sibling, 0 replies; 4+ messages in thread
From: kernel test robot @ 2024-02-18 12:58 UTC (permalink / raw
To: Robert Richter, Alison Schofield, Vishal Verma, Ira Weiny,
Dan Williams, Jonathan Cameron, Dave Jiang, Davidlohr Bueso,
Rafael J. Wysocki, Andrew Morton
Cc: llvm, oe-kbuild-all, Linux Memory Management List, linux-cxl,
linux-kernel, Len Brown, Robert Richter, linux-acpi
Hi Robert,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 6be99530c92c6b8ff7a01903edc42393575ad63b]
url: https://github.com/intel-lab-lkp/linux/commits/Robert-Richter/cxl-pci-Rename-DOE-mailbox-handle-to-doe_mb/20240217-000206
base: 6be99530c92c6b8ff7a01903edc42393575ad63b
patch link: https://lore.kernel.org/r/20240216155844.406996-4-rrichter%40amd.com
patch subject: [PATCH v4 3/3] lib/firmware_table: Provide buffer length argument to cdat_table_parse()
config: i386-randconfig-006-20240217 (https://download.01.org/0day-ci/archive/20240218/202402182055.zfTIyfls-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240218/202402182055.zfTIyfls-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402182055.zfTIyfls-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/cxl/core/pci.c:673:4: warning: format specifies type 'unsigned long' but the argument has type 'size_t' (aka 'unsigned int') [-Wformat]
672 | dev_warn(dev, "Malformed CDAT table length (%lu:%lu), discarding trailing data\n",
| ~~~
| %zu
673 | table_length, length);
| ^~~~~~~~~~~~
include/linux/dev_printk.h:146:70: note: expanded from macro 'dev_warn'
146 | dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ~~~ ^~~~~~~~~~~
include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ~~~ ^~~~~~~~~~~
drivers/cxl/core/pci.c:673:18: warning: format specifies type 'unsigned long' but the argument has type 'size_t' (aka 'unsigned int') [-Wformat]
672 | dev_warn(dev, "Malformed CDAT table length (%lu:%lu), discarding trailing data\n",
| ~~~
| %zu
673 | table_length, length);
| ^~~~~~
include/linux/dev_printk.h:146:70: note: expanded from macro 'dev_warn'
146 | dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ~~~ ^~~~~~~~~~~
include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ~~~ ^~~~~~~~~~~
2 warnings generated.
vim +673 drivers/cxl/core/pci.c
611
612 /**
613 * read_cdat_data - Read the CDAT data on this port
614 * @port: Port to read data from
615 *
616 * This call will sleep waiting for responses from the DOE mailbox.
617 */
618 void read_cdat_data(struct cxl_port *port)
619 {
620 struct device *uport = port->uport_dev;
621 struct device *dev = &port->dev;
622 struct pci_doe_mb *doe_mb;
623 struct pci_dev *pdev = NULL;
624 struct cxl_memdev *cxlmd;
625 struct cdat_doe_rsp *buf;
626 size_t table_length, length;
627 int rc;
628
629 if (is_cxl_memdev(uport)) {
630 struct device *host;
631
632 cxlmd = to_cxl_memdev(uport);
633 host = cxlmd->dev.parent;
634 if (dev_is_pci(host))
635 pdev = to_pci_dev(host);
636 } else if (dev_is_pci(uport)) {
637 pdev = to_pci_dev(uport);
638 }
639
640 if (!pdev)
641 return;
642
643 doe_mb = pci_find_doe_mailbox(pdev, PCI_DVSEC_VENDOR_ID_CXL,
644 CXL_DOE_PROTOCOL_TABLE_ACCESS);
645 if (!doe_mb) {
646 dev_dbg(dev, "No CDAT mailbox\n");
647 return;
648 }
649
650 port->cdat_available = true;
651
652 if (cxl_cdat_get_length(dev, doe_mb, &length)) {
653 dev_dbg(dev, "No CDAT length\n");
654 return;
655 }
656
657 /*
658 * The begin of the CDAT buffer needs space for additional 4
659 * bytes for the DOE header. Table data starts afterwards.
660 */
661 buf = devm_kzalloc(dev, sizeof(*buf) + length, GFP_KERNEL);
662 if (!buf)
663 goto err;
664
665 table_length = length;
666
667 rc = cxl_cdat_read_table(dev, doe_mb, buf, &length);
668 if (rc)
669 goto err;
670
671 if (table_length != length)
672 dev_warn(dev, "Malformed CDAT table length (%lu:%lu), discarding trailing data\n",
> 673 table_length, length);
674
675 if (cdat_checksum(buf->data, length))
676 goto err;
677
678 port->cdat.table = buf->data;
679 port->cdat.length = length;
680
681 return;
682 err:
683 /* Don't leave table data allocated on error */
684 devm_kfree(dev, buf);
685 dev_err(dev, "Failed to read/validate CDAT.\n");
686 }
687 EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL);
688
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v5] lib/firmware_table: Provide buffer length argument to cdat_table_parse()
2024-02-17 21:39 ` [PATCH v5] " Robert Richter
@ 2024-02-19 12:53 ` Jonathan Cameron
0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2024-02-19 12:53 UTC (permalink / raw
To: Robert Richter
Cc: Dan Williams, kernel test robot, Alison Schofield, Vishal Verma,
Ira Weiny, Dave Jiang, Davidlohr Bueso, Rafael J. Wysocki,
Andrew Morton, oe-kbuild-all, Linux Memory Management List,
linux-cxl, linux-kernel, Len Brown, linux-acpi
On Sat, 17 Feb 2024 22:39:46 +0100
Robert Richter <rrichter@amd.com> wrote:
> On 17.02.24 18:43:37, kernel test robot wrote:
> > Hi Robert,
> >
> > kernel test robot noticed the following build warnings:
> >
> > [auto build test WARNING on 6be99530c92c6b8ff7a01903edc42393575ad63b]
> >
> > url: https://github.com/intel-lab-lkp/linux/commits/Robert-Richter/cxl-pci-Rename-DOE-mailbox-handle-to-doe_mb/20240217-000206
> > base: 6be99530c92c6b8ff7a01903edc42393575ad63b
> > patch link: https://lore.kernel.org/r/20240216155844.406996-4-rrichter%40amd.com
> > patch subject: [PATCH v4 3/3] lib/firmware_table: Provide buffer length argument to cdat_table_parse()
> > config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20240217/202402171817.i0WShbft-lkp@intel.com/config)
> > compiler: arceb-elf-gcc (GCC) 13.2.0
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240217/202402171817.i0WShbft-lkp@intel.com/reproduce)
>
> > In file included from include/linux/device.h:15,
> > from drivers/cxl/core/pci.c:5:
> > drivers/cxl/core/pci.c: In function 'read_cdat_data':
> > >> drivers/cxl/core/pci.c:672:31: warning: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
> > 672 | dev_warn(dev, "Malformed CDAT table length (%lu:%lu), discarding trailing data\n",
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Fix below, it basically uses %zu for both format strings.
>
> -Robert
>
>
> From 08685053a91e370fd1263b921aa3e8942025c4e4 Mon Sep 17 00:00:00 2001
> From: Robert Richter <rrichter@amd.com>
> Date: Sun, 7 Jan 2024 18:13:16 +0100
> Subject: [PATCH v5] lib/firmware_table: Provide buffer length argument to
> cdat_table_parse()
>
> There exist card implementations with a CDAT table using a fixed size
> buffer, but with entries filled in that do not fill the whole table
> length size. Then, the last entry in the CDAT table may not mark the
> end of the CDAT table buffer specified by the length field in the CDAT
> header. It can be shorter with trailing unused (zero'ed) data. The
> actual table length is determined while reading all CDAT entries of
> the table with DOE.
>
> If the table is greater than expected (containing zero'ed trailing
> data), the CDAT parser fails with:
>
> [ 48.691717] Malformed DSMAS table length: (24:0)
> [ 48.702084] [CDAT:0x00] Invalid zero length
> [ 48.711460] cxl_port endpoint1: Failed to parse CDAT: -22
>
> In addition, a check of the table buffer length is missing to prevent
> an out-of-bound access then parsing the CDAT table.
>
> Hardening code against device returning borked table. Fix that by
> providing an optional buffer length argument to
> acpi_parse_entries_array() that can be used by cdat_table_parse() to
> propagate the buffer size down to its users to check the buffer
> length. This also prevents a possible out-of-bound access mentioned.
>
> Add a check to warn about a malformed CDAT table length.
>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Len Brown <lenb@kernel.org>
> Signed-off-by: Robert Richter <rrichter@amd.com>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Robert Richter <rrichter@amd.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-02-19 12:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20240216155844.406996-4-rrichter@amd.com>
2024-02-17 10:43 ` [PATCH v4 3/3] lib/firmware_table: Provide buffer length argument to cdat_table_parse() kernel test robot
2024-02-17 21:39 ` [PATCH v5] " Robert Richter
2024-02-19 12:53 ` Jonathan Cameron
2024-02-18 12:58 ` [PATCH v4 3/3] " kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).