linux-kernel - Re: [PATCH] platform/x86: ISST: Fix return value on last invalid resource

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] [day] [month] [year] [list]

Message-ID: <e525b1ee-da15-a265-4e12-33c68e01b78f@linux.intel.com>
Date: Tue, 20 Aug 2024 14:23:18 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
cc: Hans de Goede <hdegoede@...hat.com>, platform-driver-x86@...r.kernel.org, 
    LKML <linux-kernel@...r.kernel.org>, stable@...r.kernel.org
Subject: Re: [PATCH] platform/x86: ISST: Fix return value on last invalid
 resource

On Fri, 16 Aug 2024, Srinivas Pandruvada wrote:

> When only the last resource is invalid, tpmi_sst_dev_add() is returing
> error even if there are other valid resources before. This function
> should return error when there are no valid resources.
> 
> Here tpmi_sst_dev_add() is returning "ret" variable. But this "ret"
> variable contains the failure status of last call to sst_main(), which
> failed for the invalid resource. But there may be other valid resources
> before the last entry.
> 
> To address this, do not update "ret" variable for sst_main() return
> status.
>
> Fixes: 9d1d36268f3d ("platform/x86: ISST: Support partitioned systems")
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
> Cc: <stable@...r.kernel.org> # 6.10+

Thanks for the patch. Applied to review-ilpo.

While applying, I added the answer to the obvious question: why no new 
checks are needed for the no valid resources case (essentially, noting the 
existing !inst check).

-- 
 i.

> ---
>  drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
> index 7fa360073f6e..404582307109 100644
> --- a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
> +++ b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
> @@ -1549,8 +1549,7 @@ int tpmi_sst_dev_add(struct auxiliary_device *auxdev)
>  			goto unlock_free;
>  		}
>  
> -		ret = sst_main(auxdev, &pd_info[i]);
> -		if (ret) {
> +		if (sst_main(auxdev, &pd_info[i])) {
>  			/*
>  			 * This entry is not valid, hardware can partially
>  			 * populate dies. In this case MMIO will have 0xFFs.
>