linux-kernel - Re: [PATCH] iommu/arm-smmu-v3: Fix overflow with sid

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: <ZwSw9OK5vMKgY6Q8@google.com>
Date: Tue, 8 Oct 2024 04:11:32 +0000
From: Pranjal Shrivastava <praan@...gle.com>
To: "Aneesh Kumar K.V (Arm)" <aneesh.kumar@...nel.org>
Cc: linux-kernel@...r.kernel.org, iommu@...ts.linux.dev,
	jean-philippe@...aro.org, Nicolin Chen <nicolinc@...dia.com>,
	Jason Gunthorpe <jgg@...dia.com>, Will Deacon <will@...nel.org>
Subject: Re: [PATCH] iommu/arm-smmu-v3: Fix overflow with sid_bits when
 computing level1 table index

On Tue, Oct 08, 2024 at 08:48:31AM +0530, Aneesh Kumar K.V (Arm) wrote:

Hi Aneesh,

> As per the spec, max bits of StreamID can be a value between 0 - 32
> inclusive. With the FVP model, SMMU_IDR1.SID_SIZE returns value 32 which
> results in arm_smmu_init_strtab_2lvl computing the last_sid_idx wrongly.
> This caused a failure in ahci disk initialization with the FVP model as
> shown below.
> 
> [    7.147067] ata1.00: qc timeout after 5000 msecs (cmd 0xec)
> [    7.147177] ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4)
> [    7.458320] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
> [   17.643140] ata1.00: qc timeout after 10000 msecs (cmd 0xec)
> [   17.643251] ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4)
> [   17.643359] ata1: limiting SATA link speed to 3.0 Gbps
> [   17.954651] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 320)
> [   48.107079] ata1.00: qc timeout after 30000 msecs (cmd 0xec)
> [   48.107190] ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4)
> 
> Cc: Nicolin Chen <nicolinc@...dia.com>
> Cc: Jason Gunthorpe <jgg@...dia.com>
> Cc: Will Deacon <will@...nel.org>
> Fixes: ce410410f1a7 ("iommu/arm-smmu-v3: Add arm_smmu_strtab_l1/2_idx()")
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@...nel.org>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 737c5b882355..01a2faee04bc 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -3625,7 +3625,7 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
>  	u32 l1size;
>  	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
>  	unsigned int last_sid_idx =
> -		arm_smmu_strtab_l1_idx((1 << smmu->sid_bits) - 1);
> +		arm_smmu_strtab_l1_idx((1UL << smmu->sid_bits) - 1);

There's another patch [1] from Yang Shi for this discussion already.
Daniel had another similar patch [2] which was converged with [1].
>  
>  	/* Calculate the L1 size, capped to the SIDSIZE. */
>  	cfg->l2.num_l1_ents = min(last_sid_idx + 1, STRTAB_MAX_L1_ENTRIES);
> -- 
> 2.34.1
> 
>

[1]
 https://lore.kernel.org/linux-arm-kernel/20241001180346.1485194-1-yang@os.amperecomputing.com/

[2]
 https://lore.kernel.org/all/fd04bbc8-8ebb-4091-b56d-32072587fa99@os.amperecomputing.com/T/

Thanks,
Pranjal