Mailing List Archive

[PATCH] AMD/IOMMU: avoid UB in guest CR3 retrieval
Found by looking for patterns similar to the one Julien did spot in
pci_vtd_quirks(). (Not that it matters much here, considering the code
is dead right now.)

Fixes: 3a7947b69011 ("amd-iommu: use a bitfield for DTE")
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/drivers/passthrough/amd/iommu_guest.c
+++ b/xen/drivers/passthrough/amd/iommu_guest.c
@@ -70,7 +70,8 @@ static void guest_iommu_disable(struct g

static uint64_t get_guest_cr3_from_dte(struct amd_iommu_dte *dte)
{
- return ((dte->gcr3_trp_51_31 << 31) | (dte->gcr3_trp_30_15 << 15) |
+ return (((uint64_t)dte->gcr3_trp_51_31 << 31) |
+ (dte->gcr3_trp_30_15 << 15) |
(dte->gcr3_trp_14_12 << 12)) >> PAGE_SHIFT;
}
Re: [PATCH] AMD/IOMMU: avoid UB in guest CR3 retrieval [ In reply to ]
On 19/11/2020 15:58, Jan Beulich wrote:
> Found by looking for patterns similar to the one Julien did spot in
> pci_vtd_quirks(). (Not that it matters much here, considering the code
> is dead right now.)
>
> Fixes: 3a7947b69011 ("amd-iommu: use a bitfield for DTE")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

There is *still* an outstanding regression (modulo dead code) in one of
these bitfield-ifications which is off by 12, but I can't remember if it
is this one or not.

>
> --- a/xen/drivers/passthrough/amd/iommu_guest.c
> +++ b/xen/drivers/passthrough/amd/iommu_guest.c
> @@ -70,7 +70,8 @@ static void guest_iommu_disable(struct g
>
> static uint64_t get_guest_cr3_from_dte(struct amd_iommu_dte *dte)
> {
> - return ((dte->gcr3_trp_51_31 << 31) | (dte->gcr3_trp_30_15 << 15) |
> + return (((uint64_t)dte->gcr3_trp_51_31 << 31) |
> + (dte->gcr3_trp_30_15 << 15) |
> (dte->gcr3_trp_14_12 << 12)) >> PAGE_SHIFT;
> }
>