linux-kernel - Re: [PATCH rc v5 1/4] iommu/arm-smmu-v3: Add update

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: <aWZfUNXEka3GBf9s@willie-the-truck>
Date: Tue, 13 Jan 2026 15:05:52 +0000
From: Will Deacon <will@...nel.org>
To: Nicolin Chen <nicolinc@...dia.com>
Cc: Jason Gunthorpe <jgg@...dia.com>, robin.murphy@....com, joro@...tes.org,
	linux-arm-kernel@...ts.infradead.org, iommu@...ts.linux.dev,
	linux-kernel@...r.kernel.org, skolothumtho@...dia.com,
	praan@...gle.com, xueshuai@...ux.alibaba.com, smostafa@...gle.com
Subject: Re: [PATCH rc v5 1/4] iommu/arm-smmu-v3: Add update_safe bits to fix
 STE update sequence

On Mon, Jan 12, 2026 at 10:58:22AM -0800, Nicolin Chen wrote:
> On Mon, Jan 12, 2026 at 12:10:10PM -0400, Jason Gunthorpe wrote:
> > Still, it seems easy enough to improve, do not add EATS to the safe
> > bits if either the current or new STE has S2S set. That will force a
> > V=0 and avoid the illegal STE. Nicolin?
> 
> Ack. I made the following changes:
> -----------------------------------------------------------------
> @@ -1083,7 +1083,8 @@ void arm_smmu_get_ste_used(const __le64 *ent, __le64 *used_bits)
>  EXPORT_SYMBOL_IF_KUNIT(arm_smmu_get_ste_used);
> 
>  VISIBLE_IF_KUNIT
> -void arm_smmu_get_ste_update_safe(__le64 *safe_bits)
> +void arm_smmu_get_ste_update_safe(const __le64 *cur, const __le64 *target,
> +                                 __le64 *safe_bits)
>  {
>         /*
>          * MEV does not meaningfully impact the operation of the HW, it only
> @@ -1097,13 +1098,22 @@ void arm_smmu_get_ste_update_safe(__le64 *safe_bits)
>         safe_bits[1] |= cpu_to_le64(STRTAB_STE_1_MEV);
> 
>         /*
> -        * EATS is used to reject and control the ATS behavior of the device. If
> -        * we are changing it away from 0 then we already trust the device to
> -        * use ATS properly and we have sequenced the device's ATS enable in PCI
> -        * config space to prevent it from issuing ATS while we are changing
> -        * EATS.
> +        * When a STE comes to change EATS the sequencing code in the attach
> +        * logic already will have the PCI cap for ATS disabled. Thus at this
> +        * moment we can expect that the device will not generate ATS queries
> +        * and so we don't care about the sequencing of EATS. The purpose of
> +        * EATS is to protect the system from hostile untrusted devices that
> +        * issue ATS when the PCI config space is disabled. However, if EATS
> +        * is being changed then we already must be trusting the device since
> +        * the EATS security block is being disabled.
> +        *
> +        *  Note: Since we moved the EATS update to the first phase, changing
> +        *  S2S and EATS might transiently set S2S=1 and EATS=1, resulting in
> +        *  a bad STE. See "5.2 Stream Table Entry". In such a case, we can't
> +        *  do a hitless update.
>          */
> -       safe_bits[1] |= cpu_to_le64(STRTAB_STE_1_EATS);
> +       if (!((cur[2] | target[2]) & cpu_to_le64(STRTAB_STE_2_S2S)))
> +               safe_bits[1] |= cpu_to_le64(STRTAB_STE_1_EATS);

I suppose we shouldn't ever see the case that they both have S2S, but
that's fine.

The spec also suggests that there's an additional illegal STE case w/
split-stage ATS (EATS_S1CHK) if Config != S1+S2.

I do wonder whether having all the hitless machinery alongside this
"safe" stuff is really overkill and we wouldn't be better off just
checking the cases that we actually care about rather than checking
architecturally and then subtracting the cases we don't care about.

Will