On Sat, 17 Feb 2024 22:39:46 +0100 Robert Richter <rrichter@xxxxxxx> 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@xxxxxxxxx/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@xxxxxxxxx/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@xxxxxxx> > 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@xxxxxxxxxx> > Cc: Len Brown <lenb@xxxxxxxxxx> > Signed-off-by: Robert Richter <rrichter@xxxxxxx> > Reviewed-by: Dave Jiang <dave.jiang@xxxxxxxxx> > Signed-off-by: Robert Richter <rrichter@xxxxxxx> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>