2020-09-10 17:20:19

by Joao Martins

Subject: [PATCH] iommu/amd: Fix potential @entry null deref

After commit 26e495f34107 ("iommu/amd: Restore IRTE.RemapEn bit after
programming IRTE"), smatch warns:

drivers/iommu/amd/iommu.c:3870 amd_iommu_deactivate_guest_mode()
warn: variable dereferenced before check 'entry' (see line 3867)

Fix this by moving the @valid assignment to after @entry has been checked
for NULL.

Cc: Suravee Suthikulpanit <[email protected]>
Fixes: 26e495f34107 ("iommu/amd: Restore IRTE.RemapEn bit after programming IRTE")
Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: Joao Martins <[email protected]>
---
drivers/iommu/amd/iommu.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 07ae8b93887e..8abe1c7ad45b 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -3864,12 +3864,14 @@ int amd_iommu_deactivate_guest_mode(void *data)
struct amd_ir_data *ir_data = (struct amd_ir_data *)data;
struct irte_ga *entry = (struct irte_ga *) ir_data->entry;
struct irq_cfg *cfg = ir_data->cfg;
- u64 valid = entry->lo.fields_remap.valid;
+ u64 valid;

if (!AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir) ||
!entry || !entry->lo.fields_vapic.guest_mode)
return 0;

+ valid = entry->lo.fields_remap.valid;
+
entry->lo.val = 0;
entry->hi.val = 0;

--
2.17.1

2020-09-11 03:27:54

by Suthikulpanit, Suravee

Subject: Re: [PATCH] iommu/amd: Fix potential @entry null deref

Thanks for catching this.

On 9/11/20 12:16 AM, Joao Martins wrote:
> After commit 26e495f34107 ("iommu/amd: Restore IRTE.RemapEn bit after
> programming IRTE"), smatch warns:
>
> drivers/iommu/amd/iommu.c:3870 amd_iommu_deactivate_guest_mode()
> warn: variable dereferenced before check 'entry' (see line 3867)
>
> Fix this by moving the @valid assignment to after @entry has been checked
> for NULL.
>
> Cc: Suravee Suthikulpanit <[email protected]>
> Fixes: 26e495f34107 ("iommu/amd: Restore IRTE.RemapEn bit after programming IRTE")
> Reported-by: Dan Carpenter <[email protected]>
> Signed-off-by: Joao Martins <[email protected]>
> ---
> drivers/iommu/amd/iommu.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 07ae8b93887e..8abe1c7ad45b 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -3864,12 +3864,14 @@ int amd_iommu_deactivate_guest_mode(void *data)
> struct amd_ir_data *ir_data = (struct amd_ir_data *)data;
> struct irte_ga *entry = (struct irte_ga *) ir_data->entry;
> struct irq_cfg *cfg = ir_data->cfg;
> - u64 valid = entry->lo.fields_remap.valid;
> + u64 valid;
>
> if (!AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir) ||
> !entry || !entry->lo.fields_vapic.guest_mode)
> return 0;
>
> + valid = entry->lo.fields_remap.valid;
> +
> entry->lo.val = 0;
> entry->hi.val = 0;
>
>

Reviewed-by: Suravee Suthikulpanit <[email protected]>

Suravee

2020-09-18 08:37:45

by Joerg Roedel

Subject: Re: [PATCH] iommu/amd: Fix potential @entry null deref

On Thu, Sep 10, 2020 at 06:16:21PM +0100, Joao Martins wrote:
> After commit 26e495f34107 ("iommu/amd: Restore IRTE.RemapEn bit after
> programming IRTE"), smatch warns:
>
> drivers/iommu/amd/iommu.c:3870 amd_iommu_deactivate_guest_mode()
> warn: variable dereferenced before check 'entry' (see line 3867)
>
> Fix this by moving the @valid assignment to after @entry has been checked
> for NULL.
>
> Cc: Suravee Suthikulpanit <[email protected]>
> Fixes: 26e495f34107 ("iommu/amd: Restore IRTE.RemapEn bit after programming IRTE")
> Reported-by: Dan Carpenter <[email protected]>
> Signed-off-by: Joao Martins <[email protected]>
> ---
> drivers/iommu/amd/iommu.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)

Applied for v5.9, thanks.