| 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
| ||
|
Message-ID: <2e160dfa-9c01-28e1-65e3-029cb29c9c62@linux.intel.com>
Date: Wed, 10 Apr 2019 11:05:48 -0700
From: sathyanarayanan kuppuswamy
<sathyanarayanan.kuppuswamy@...ux.intel.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Darren Hart <dvhart@...radead.org>,
platform-driver-x86@...r.kernel.org,
Zha Qipeng <qipeng.zha@...el.com>, junxiao.chang@...el.com,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 3/4] platform/x86: intel_pmc_ipc: Don't map non-used
optional resources
Hi,
On 4/9/19 4:25 AM, Andy Shevchenko wrote:
> The intel_pmc_ipc driver has a placeholder for all possible resources
> that may have been provided by ACPI. Since there are few optional ones,
> the driver still uses them and binds to wrong ranges in resource tree:
>
> # grep intel_punit_ipc /proc/iomem
> 00000000-00000000 : intel_punit_ipc
> 00000000-00000000 : intel_punit_ipc
> 00000000-00000000 : intel_punit_ipc
> 00000000-00000000 : intel_punit_ipc
>
> This leads to issues with resource management during inserting and
> removing modules, such as intel_pmc_ipc itself, which can't be inserted
> anymore after first removal.
>
> Count the actual resources provided and supply only them to the child device.
>
> This is a real fix of the commit 8cc7fb4a6523
>
> ("intel_pmc_ipc: update acpi resource structure for Punit")
>
> that also fixes a symptoms described in the commit 6cc8cbbc8868
>
> ("platform/x86: intel_punit_ipc: Fix resource ioremap warning")
>
> that is going to be reverted afterwards.
Looks good to me.
Reviewed-by: Kuppuswamy Sathyanarayanan
<sathyanarayanan.kuppuswamy@...ux.intel.com>
>
> Reported-by: Junxiao Chang <junxiao.chang@...el.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> Cc: Qipeng Zha <qipeng.zha@...el.com>
> Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
> ---
> drivers/platform/x86/intel_pmc_ipc.c | 24 +++++++++++-------------
> 1 file changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
> index 9007aa717586..55037ff258f8 100644
> --- a/drivers/platform/x86/intel_pmc_ipc.c
> +++ b/drivers/platform/x86/intel_pmc_ipc.c
> @@ -131,6 +131,7 @@ static struct intel_pmc_ipc_dev {
>
> /* punit */
> struct platform_device *punit_dev;
> + unsigned int punit_res_count;
>
> /* Telemetry */
> resource_size_t telem_pmc_ssram_base;
> @@ -682,7 +683,7 @@ static int ipc_create_punit_device(void)
> .name = PUNIT_DEVICE_NAME,
> .id = -1,
> .res = punit_res_array,
> - .num_res = ARRAY_SIZE(punit_res_array),
> + .num_res = ipcdev.punit_res_count,
> };
>
> pdev = platform_device_register_full(&pdevinfo);
> @@ -789,7 +790,7 @@ static int ipc_create_pmc_devices(void)
>
> static int ipc_plat_get_res(struct platform_device *pdev)
> {
> - struct resource *res, *punit_res;
> + struct resource *res, *punit_res = punit_res_array;
> void __iomem *addr;
> int size;
>
> @@ -804,7 +805,8 @@ static int ipc_plat_get_res(struct platform_device *pdev)
> ipcdev.acpi_io_size = size;
> dev_info(&pdev->dev, "io res: %pR\n", res);
>
> - punit_res = punit_res_array;
> + ipcdev.punit_res_count = 0;
> +
> /* This is index 0 to cover BIOS data register */
> res = platform_get_resource(pdev, IORESOURCE_MEM,
> PLAT_RESOURCE_BIOS_DATA_INDEX);
> @@ -812,7 +814,7 @@ static int ipc_plat_get_res(struct platform_device *pdev)
> dev_err(&pdev->dev, "Failed to get res of punit BIOS data\n");
> return -ENXIO;
> }
> - *punit_res = *res;
> + punit_res[ipcdev.punit_res_count++] = *res;
> dev_info(&pdev->dev, "punit BIOS data res: %pR\n", res);
>
> /* This is index 1 to cover BIOS interface register */
> @@ -822,42 +824,38 @@ static int ipc_plat_get_res(struct platform_device *pdev)
> dev_err(&pdev->dev, "Failed to get res of punit BIOS iface\n");
> return -ENXIO;
> }
> - *++punit_res = *res;
> + punit_res[ipcdev.punit_res_count++] = *res;
> dev_info(&pdev->dev, "punit BIOS interface res: %pR\n", res);
>
> /* This is index 2 to cover ISP data register, optional */
> res = platform_get_resource(pdev, IORESOURCE_MEM,
> PLAT_RESOURCE_ISP_DATA_INDEX);
> - ++punit_res;
> if (res) {
> - *punit_res = *res;
> + punit_res[ipcdev.punit_res_count++] = *res;
> dev_info(&pdev->dev, "punit ISP data res: %pR\n", res);
> }
>
> /* This is index 3 to cover ISP interface register, optional */
> res = platform_get_resource(pdev, IORESOURCE_MEM,
> PLAT_RESOURCE_ISP_IFACE_INDEX);
> - ++punit_res;
> if (res) {
> - *punit_res = *res;
> + punit_res[ipcdev.punit_res_count++] = *res;
> dev_info(&pdev->dev, "punit ISP interface res: %pR\n", res);
> }
>
> /* This is index 4 to cover GTD data register, optional */
> res = platform_get_resource(pdev, IORESOURCE_MEM,
> PLAT_RESOURCE_GTD_DATA_INDEX);
> - ++punit_res;
> if (res) {
> - *punit_res = *res;
> + punit_res[ipcdev.punit_res_count++] = *res;
> dev_info(&pdev->dev, "punit GTD data res: %pR\n", res);
> }
>
> /* This is index 5 to cover GTD interface register, optional */
> res = platform_get_resource(pdev, IORESOURCE_MEM,
> PLAT_RESOURCE_GTD_IFACE_INDEX);
> - ++punit_res;
> if (res) {
> - *punit_res = *res;
> + punit_res[ipcdev.punit_res_count++] = *res;
> dev_info(&pdev->dev, "punit GTD interface res: %pR\n", res);
> }
>
--
Sathyanarayanan Kuppuswamy
Linux kernel developer
Powered by blists - more mailing lists