linux-kernel - Re: [PATCH] testing/cxl: Fix abused pci_bus_read_config

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6711b7c0c0b53_3ee2294a6@dwillia2-xfh.jf.intel.com.notmuch>
Date: Thu, 17 Oct 2024 18:20:00 -0700
From: Dan Williams <dan.j.williams@...el.com>
To: Alison Schofield <alison.schofield@...el.com>, Li Zhijian
	<lizhijian@...itsu.com>
CC: <linux-cxl@...r.kernel.org>, Davidlohr Bueso <dave@...olabs.net>, Jonathan
 Cameron <jonathan.cameron@...wei.com>, Dave Jiang <dave.jiang@...el.com>,
	Vishal Verma <vishal.l.verma@...el.com>, Ira Weiny <ira.weiny@...el.com>, Dan
 Williams <dan.j.williams@...el.com>, <linux-kernel@...r.kernel.org>, "Huang,
 Ying" <ying.huang@...el.com>
Subject: Re: [PATCH] testing/cxl: Fix abused pci_bus_read_config_word() on
 platform device

Alison Schofield wrote:
> On Wed, Oct 16, 2024 at 09:52:13AM +0800, Li Zhijian wrote:
> > The cxl_region_shared_upstream_bandwidth_update() in clx_core works on
> > PCI/PCIe CXL device only while cxl_test was implemeneted by platform
> > device.
> > 
> > Mock a cxl_region_shared_upstream_bandwidth_update() which does nothing
> > for cxl_core so that the cxl_test goes well.
> > 
> > Abuse cxl_region_shared_upstream_bandwidth_update() on platform device
> > will cause a kernel panic with calltrace:
> 
> snip
> 
> > ---
> >  tools/testing/cxl/Kbuild      | 2 ++
> >  tools/testing/cxl/mock_cdat.c | 8 ++++++++
> >  2 files changed, 10 insertions(+)
> >  create mode 100644 tools/testing/cxl/mock_cdat.c
> > 
> > diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
> > index b1256fee3567..ed9f50dee3f5 100644
> > --- a/tools/testing/cxl/Kbuild
> > +++ b/tools/testing/cxl/Kbuild
> > @@ -15,6 +15,7 @@ ldflags-y += --wrap=devm_cxl_add_rch_dport
> >  ldflags-y += --wrap=cxl_rcd_component_reg_phys
> >  ldflags-y += --wrap=cxl_endpoint_parse_cdat
> >  ldflags-y += --wrap=cxl_dport_init_ras_reporting
> > +ldflags-y += --wrap=cxl_region_shared_upstream_bandwidth_update
> >  
> >  DRIVERS := ../../../drivers
> >  CXL_SRC := $(DRIVERS)/cxl
> > @@ -61,6 +62,7 @@ cxl_core-y += $(CXL_CORE_SRC)/pci.o
> >  cxl_core-y += $(CXL_CORE_SRC)/hdm.o
> >  cxl_core-y += $(CXL_CORE_SRC)/pmu.o
> >  cxl_core-y += $(CXL_CORE_SRC)/cdat.o
> > +cxl_core-y += mock_cdat.o
> >  cxl_core-$(CONFIG_TRACING) += $(CXL_CORE_SRC)/trace.o
> >  cxl_core-$(CONFIG_CXL_REGION) += $(CXL_CORE_SRC)/region.o
> >  cxl_core-y += config_check.o
> > diff --git a/tools/testing/cxl/mock_cdat.c b/tools/testing/cxl/mock_cdat.c
> > new file mode 100644
> > index 000000000000..99974153b3f6
> > --- /dev/null
> > +++ b/tools/testing/cxl/mock_cdat.c
> > @@ -0,0 +1,8 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/* Copyright (c) 2024 FUJITSU LIMITED. All rights reserved. */
> > +
> > +#include <cxl.h>
> > +
> > +void __wrap_cxl_region_shared_upstream_bandwidth_update(struct cxl_region *cxlr)
> > +{
> > +}
> 
> The addition of file mock_cdat.c made me wonder why this wrapper couldn't join
> all the other __wrap's defined in test/mock.c. I tried, as you probably did,
> and see the circular dependency. I mention it here in case anyone else has a
> way to simplify this.

Yeah, unfortunately symbols can only be mocked across EXPORT_SYMBOL()
boundaries, but since the caller of this is internal to the CXL core it
is not amenable to the wrap approach.

So, unfortunately what this patch does is break the expectation that
cxl_test can live alongside and not regress any production flows. I.e.
what this patch does is replace 
cxl_region_shared_upstream_bandwidth_update() for all use cases, not
just platform devices.

Compare that to tools/testing/cxl/test/mock.c which arranges for all the
mocked use cases to call back into the real routines in the real device
case.

Given that I think this puts the device type check back in play.

However, instead of checking "dev_is_platform()" check "dev_is_pci()" to
be consistent with all the other CXL core internal functions that exit
early when passed cxl_test devices.