From a35f2ef3b7376bfd0a57f7844bd7454389aae1fc Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Mon, 15 Feb 2021 08:49:34 +0100 Subject: Xen/x86: don't bail early from clear_foreign_p2m_mapping() Its sibling (set_foreign_p2m_mapping()) as well as the sibling of its only caller (gnttab_map_refs()) don't clean up after themselves in case of error. Higher level callers are expected to do so. However, in order for that to really clean up any partially set up state, the operation should not terminate upon encountering an entry in unexpected state. It is particularly relevant to notice here that set_foreign_p2m_mapping() would skip setting up a p2m entry if its grant mapping failed, but it would continue to set up further p2m entries as long as their mappings succeeded. Arguably down the road set_foreign_p2m_mapping() may want its page state related WARN_ON() also converted to an error return. This is part of XSA-361. Signed-off-by: Jan Beulich Cc: stable@vger.kernel.org Reviewed-by: Juergen Gross Signed-off-by: Juergen Gross --- arch/x86/xen/p2m.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'arch') diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c index 3301875dd196..ac579c44dd28 100644 --- a/arch/x86/xen/p2m.c +++ b/arch/x86/xen/p2m.c @@ -750,17 +750,15 @@ int clear_foreign_p2m_mapping(struct gnttab_unmap_grant_ref *unmap_ops, unsigned long mfn = __pfn_to_mfn(page_to_pfn(pages[i])); unsigned long pfn = page_to_pfn(pages[i]); - if (mfn == INVALID_P2M_ENTRY || !(mfn & FOREIGN_FRAME_BIT)) { + if (mfn != INVALID_P2M_ENTRY && (mfn & FOREIGN_FRAME_BIT)) + set_phys_to_machine(pfn, INVALID_P2M_ENTRY); + else ret = -EINVAL; - goto out; - } - - set_phys_to_machine(pfn, INVALID_P2M_ENTRY); } if (kunmap_ops) ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, - kunmap_ops, count); -out: + kunmap_ops, count) ?: ret; + return ret; } EXPORT_SYMBOL_GPL(clear_foreign_p2m_mapping); -- cgit 1.4.1 From b512e1b077e5ccdbd6e225b15d934ab12453b70a Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Mon, 15 Feb 2021 08:50:08 +0100 Subject: Xen/x86: also check kernel mapping in set_foreign_p2m_mapping() We should not set up further state if either mapping failed; paying attention to just the user mapping's status isn't enough. Also use GNTST_okay instead of implying its value (zero). This is part of XSA-361. Signed-off-by: Jan Beulich Cc: stable@vger.kernel.org Reviewed-by: Juergen Gross Signed-off-by: Juergen Gross --- arch/x86/xen/p2m.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'arch') diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c index ac579c44dd28..b5949e5a83ec 100644 --- a/arch/x86/xen/p2m.c +++ b/arch/x86/xen/p2m.c @@ -712,7 +712,8 @@ int set_foreign_p2m_mapping(struct gnttab_map_grant_ref *map_ops, unsigned long mfn, pfn; /* Do not add to override if the map failed. */ - if (map_ops[i].status) + if (map_ops[i].status != GNTST_okay || + (kmap_ops && kmap_ops[i].status != GNTST_okay)) continue; if (map_ops[i].flags & GNTMAP_contains_pte) { -- cgit 1.4.1 From 36bf1dfb8b266e089afa9b7b984217f17027bf35 Mon Sep 17 00:00:00 2001 From: Stefano Stabellini Date: Mon, 15 Feb 2021 08:53:44 +0100 Subject: xen/arm: don't ignore return errors from set_phys_to_machine set_phys_to_machine can fail due to lack of memory, see the kzalloc call in arch/arm/xen/p2m.c:__set_phys_to_machine_multi. Don't ignore the potential return error in set_foreign_p2m_mapping, returning it to the caller instead. This is part of XSA-361. Signed-off-by: Stefano Stabellini Cc: stable@vger.kernel.org Reviewed-by: Julien Grall Signed-off-by: Juergen Gross --- arch/arm/xen/p2m.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'arch') diff --git a/arch/arm/xen/p2m.c b/arch/arm/xen/p2m.c index e52950a43f2e..fd6e3aafe272 100644 --- a/arch/arm/xen/p2m.c +++ b/arch/arm/xen/p2m.c @@ -95,8 +95,10 @@ int set_foreign_p2m_mapping(struct gnttab_map_grant_ref *map_ops, for (i = 0; i < count; i++) { if (map_ops[i].status) continue; - set_phys_to_machine(map_ops[i].host_addr >> XEN_PAGE_SHIFT, - map_ops[i].dev_bus_addr >> XEN_PAGE_SHIFT); + if (unlikely(!set_phys_to_machine(map_ops[i].host_addr >> XEN_PAGE_SHIFT, + map_ops[i].dev_bus_addr >> XEN_PAGE_SHIFT))) { + return -ENOMEM; + } } return 0; -- cgit 1.4.1