linux-kernel - Re: [PATCH 3/3] i2c: i801: Do not add ICH_RES_IO

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: <20200225144047.GC2667@lahna.fi.intel.com>
Date:   Tue, 25 Feb 2020 16:40:47 +0200
From:   Mika Westerberg <mika.westerberg@...ux.intel.com>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     Jean Delvare <jdelvare@...e.com>,
        Wim Van Sebroeck <wim@...ux-watchdog.org>,
        Wolfram Sang <wsa@...-dreams.de>,
        Martin Volf <martin.volf.42@...il.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Jarkko Nikula <jarkko.nikula@...ux.intel.com>,
        linux-i2c@...r.kernel.org, linux-hwmon@...r.kernel.org,
        linux-watchdog@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] i2c: i801: Do not add ICH_RES_IO_SMI if PMC device
 is not present

On Tue, Feb 25, 2020 at 06:21:16AM -0800, Guenter Roeck wrote:
> On 2/25/20 4:38 AM, Mika Westerberg wrote:
> > Martin noticed that nct6775 driver does not load properly on his system
> > in v5.4+ kernels. The issue was bisected to commit b84398d6d7f9 ("i2c:
> > i801: Use iTCO version 6 in Cannon Lake PCH and beyond") but it is
> > likely not the culprit because the faulty code has been in the driver
> > already since commit 9424693035a5 ("i2c: i801: Create iTCO device on
> > newer Intel PCHs"). So more likely some commit that added PCI IDs of
> > recent chipsets made the driver to create the iTCO_wdt device on Martins
> > system.
> > 
> > The issue was debugged to be PCI configuration access to the PMC device
> > that is not present. This returns all 1's when read and this caused the
> > iTCO_wdt driver to accidentally request resourses used by nct6775.
> > 
> > Fix this by checking that the PMC device is there and only then populate
> > the iTCO_wdt ICH_RES_IO_SMI resource. Since the resource is now optional
> > the iTCO_wdt driver should continue to work on recent systems without it.
> > 
> > Link: https://lore.kernel.org/linux-hwmon/CAM1AHpQ4196tyD=HhBu-2donSsuogabkfP03v1YF26Q7_BgvgA@mail.gmail.com/
> > Fixes: 9424693035a5 ("i2c: i801: Create iTCO device on newer Intel PCHs")
> > Reported-by: Martin Volf <martin.volf.42@...il.com>
> > Signed-off-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
> > ---
> >   drivers/i2c/busses/i2c-i801.c | 45 +++++++++++++++++++++--------------
> >   1 file changed, 27 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> > index ca4f096fef74..7fa58375bd4b 100644
> > --- a/drivers/i2c/busses/i2c-i801.c
> > +++ b/drivers/i2c/busses/i2c-i801.c
> > @@ -1519,7 +1519,7 @@ static DEFINE_SPINLOCK(p2sb_spinlock);
> >   static struct platform_device *
> >   i801_add_tco_spt(struct i801_priv *priv, struct pci_dev *pci_dev,
> > -		 struct resource *tco_res)
> > +		 struct resource *tco_res, size_t nres)
> >   {
> >   	struct resource *res;
> >   	unsigned int devfn;
> > @@ -1563,7 +1563,7 @@ i801_add_tco_spt(struct i801_priv *priv, struct pci_dev *pci_dev,
> >   	res->flags = IORESOURCE_MEM;
> >   	return platform_device_register_resndata(&pci_dev->dev, "iTCO_wdt", -1,
> > -					tco_res, 3, &spt_tco_platform_data,
> > +					tco_res, nres + 1, &spt_tco_platform_data,
> 
> Does this work as intended ? It still adds ICH_RES_MEM_OFF at index 2,
> but if there is no SMI resource it will only pass two sets of resources
> to the wdt driver, one of which (the SMI resource) would be empty,
> ie have start == NULL and size == 0.

Good point that would not work as expected. I'll fix this one in the
next version.