From 032c1b698be8ed9f635ebc55d92ef5bb82c6716b Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Mon, 22 Feb 2021 10:39:06 +0100 Subject: [PATCH] 4.9-stable patches added patches: xen-arm-don-t-ignore-return-errors-from-set_phys_to_machine.patch xen-blkback-don-t-handle-error-by-bug.patch xen-blkback-fix-error-handling-in-xen_blkbk_map.patch xen-gntdev-correct-dev_bus_addr-handling-in-gntdev_map_grant_pages.patch xen-gntdev-correct-error-checking-in-gntdev_map_grant_pages.patch xen-netback-don-t-handle-error-by-bug.patch xen-scsiback-don-t-handle-error-by-bug.patch xen-x86-also-check-kernel-mapping-in-set_foreign_p2m_mapping.patch xen-x86-don-t-bail-early-from-clear_foreign_p2m_mapping.patch --- queue-4.9/series | 9 +++ ...turn-errors-from-set_phys_to_machine.patch | 42 ++++++++++ ...en-blkback-don-t-handle-error-by-bug.patch | 50 ++++++++++++ ...-fix-error-handling-in-xen_blkbk_map.patch | 78 +++++++++++++++++++ ...r-handling-in-gntdev_map_grant_pages.patch | 75 ++++++++++++++++++ ...r-checking-in-gntdev_map_grant_pages.patch | 77 ++++++++++++++++++ ...en-netback-don-t-handle-error-by-bug.patch | 44 +++++++++++ ...n-scsiback-don-t-handle-error-by-bug.patch | 44 +++++++++++ ...l-mapping-in-set_foreign_p2m_mapping.patch | 38 +++++++++ ...early-from-clear_foreign_p2m_mapping.patch | 59 ++++++++++++++ 10 files changed, 516 insertions(+) create mode 100644 queue-4.9/xen-arm-don-t-ignore-return-errors-from-set_phys_to_machine.patch create mode 100644 queue-4.9/xen-blkback-don-t-handle-error-by-bug.patch create mode 100644 queue-4.9/xen-blkback-fix-error-handling-in-xen_blkbk_map.patch create mode 100644 queue-4.9/xen-gntdev-correct-dev_bus_addr-handling-in-gntdev_map_grant_pages.patch create mode 100644 queue-4.9/xen-gntdev-correct-error-checking-in-gntdev_map_grant_pages.patch create mode 100644 queue-4.9/xen-netback-don-t-handle-error-by-bug.patch create mode 100644 queue-4.9/xen-scsiback-don-t-handle-error-by-bug.patch create mode 100644 queue-4.9/xen-x86-also-check-kernel-mapping-in-set_foreign_p2m_mapping.patch create mode 100644 queue-4.9/xen-x86-don-t-bail-early-from-clear_foreign_p2m_mapping.patch diff --git a/queue-4.9/series b/queue-4.9/series index 8326ec2a7f2..ae95cd09fac 100644 --- a/queue-4.9/series +++ b/queue-4.9/series @@ -36,3 +36,12 @@ x86-build-disable-cet-instrumentation-in-the-kernel-for-32-bit-too.patch trace-use-mcount-record-for-dynamic-ftrace.patch tracing-fix-skip_stack_validation-1-build-due-to-bad-merge-with-mrecord-mcount.patch tracing-avoid-calling-cc-option-mrecord-mcount-for-every-makefile.patch +xen-x86-don-t-bail-early-from-clear_foreign_p2m_mapping.patch +xen-x86-also-check-kernel-mapping-in-set_foreign_p2m_mapping.patch +xen-gntdev-correct-dev_bus_addr-handling-in-gntdev_map_grant_pages.patch +xen-gntdev-correct-error-checking-in-gntdev_map_grant_pages.patch +xen-arm-don-t-ignore-return-errors-from-set_phys_to_machine.patch +xen-blkback-don-t-handle-error-by-bug.patch +xen-netback-don-t-handle-error-by-bug.patch +xen-scsiback-don-t-handle-error-by-bug.patch +xen-blkback-fix-error-handling-in-xen_blkbk_map.patch diff --git a/queue-4.9/xen-arm-don-t-ignore-return-errors-from-set_phys_to_machine.patch b/queue-4.9/xen-arm-don-t-ignore-return-errors-from-set_phys_to_machine.patch new file mode 100644 index 00000000000..f08ac03d876 --- /dev/null +++ b/queue-4.9/xen-arm-don-t-ignore-return-errors-from-set_phys_to_machine.patch @@ -0,0 +1,42 @@ +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 + +From: Stefano Stabellini + +commit 36bf1dfb8b266e089afa9b7b984217f17027bf35 upstream. + +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 +Signed-off-by: Greg Kroah-Hartman + +--- + arch/arm/xen/p2m.c | 6 ++++-- + 1 file changed, 4 insertions(+), 2 deletions(-) + +--- a/arch/arm/xen/p2m.c ++++ b/arch/arm/xen/p2m.c +@@ -93,8 +93,10 @@ int set_foreign_p2m_mapping(struct gntta + 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; diff --git a/queue-4.9/xen-blkback-don-t-handle-error-by-bug.patch b/queue-4.9/xen-blkback-don-t-handle-error-by-bug.patch new file mode 100644 index 00000000000..af3132bbb6d --- /dev/null +++ b/queue-4.9/xen-blkback-don-t-handle-error-by-bug.patch @@ -0,0 +1,50 @@ +From 5a264285ed1cd32e26d9de4f3c8c6855e467fd63 Mon Sep 17 00:00:00 2001 +From: Jan Beulich +Date: Mon, 15 Feb 2021 08:54:51 +0100 +Subject: xen-blkback: don't "handle" error by BUG() + +From: Jan Beulich + +commit 5a264285ed1cd32e26d9de4f3c8c6855e467fd63 upstream. + +In particular -ENOMEM may come back here, from set_foreign_p2m_mapping(). +Don't make problems worse, the more that handling elsewhere (together +with map's status fields now indicating whether a mapping wasn't even +attempted, and hence has to be considered failed) doesn't require this +odd way of dealing with errors. + +This is part of XSA-362. + +Signed-off-by: Jan Beulich +Cc: stable@vger.kernel.org +Reviewed-by: Juergen Gross +Signed-off-by: Juergen Gross +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/block/xen-blkback/blkback.c | 6 ++---- + 1 file changed, 2 insertions(+), 4 deletions(-) + +--- a/drivers/block/xen-blkback/blkback.c ++++ b/drivers/block/xen-blkback/blkback.c +@@ -860,10 +860,8 @@ again: + break; + } + +- if (segs_to_map) { ++ if (segs_to_map) + ret = gnttab_map_refs(map, NULL, pages_to_gnt, segs_to_map); +- BUG_ON(ret); +- } + + /* + * Now swizzle the MFN in our domain with the MFN from the other domain +@@ -878,7 +876,7 @@ again: + pr_debug("invalid buffer -- could not remap it\n"); + put_free_pages(ring, &pages[seg_idx]->page, 1); + pages[seg_idx]->handle = BLKBACK_INVALID_HANDLE; +- ret |= 1; ++ ret |= !ret; + goto next; + } + pages[seg_idx]->handle = map[new_map_idx].handle; diff --git a/queue-4.9/xen-blkback-fix-error-handling-in-xen_blkbk_map.patch b/queue-4.9/xen-blkback-fix-error-handling-in-xen_blkbk_map.patch new file mode 100644 index 00000000000..8beb6fa2843 --- /dev/null +++ b/queue-4.9/xen-blkback-fix-error-handling-in-xen_blkbk_map.patch @@ -0,0 +1,78 @@ +From 871997bc9e423f05c7da7c9178e62dde5df2a7f8 Mon Sep 17 00:00:00 2001 +From: Jan Beulich +Date: Mon, 15 Feb 2021 08:56:44 +0100 +Subject: xen-blkback: fix error handling in xen_blkbk_map() + +From: Jan Beulich + +commit 871997bc9e423f05c7da7c9178e62dde5df2a7f8 upstream. + +The function uses a goto-based loop, which may lead to an earlier error +getting discarded by a later iteration. Exit this ad-hoc loop when an +error was encountered. + +The out-of-memory error path additionally fails to fill a structure +field looked at by xen_blkbk_unmap_prepare() before inspecting the +handle which does get properly set (to BLKBACK_INVALID_HANDLE). + +Since the earlier exiting from the ad-hoc loop requires the same field +filling (invalidation) as that on the out-of-memory path, fold both +paths. While doing so, drop the pr_alert(), as extra log messages aren't +going to help the situation (the kernel will log oom conditions already +anyway). + +This is XSA-365. + +Signed-off-by: Jan Beulich +Reviewed-by: Juergen Gross +Reviewed-by: Julien Grall +Signed-off-by: Juergen Gross +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/block/xen-blkback/blkback.c | 24 ++++++++++++++---------- + 1 file changed, 14 insertions(+), 10 deletions(-) + +--- a/drivers/block/xen-blkback/blkback.c ++++ b/drivers/block/xen-blkback/blkback.c +@@ -843,8 +843,11 @@ again: + pages[i]->page = persistent_gnt->page; + pages[i]->persistent_gnt = persistent_gnt; + } else { +- if (get_free_page(ring, &pages[i]->page)) +- goto out_of_memory; ++ if (get_free_page(ring, &pages[i]->page)) { ++ put_free_pages(ring, pages_to_gnt, segs_to_map); ++ ret = -ENOMEM; ++ goto out; ++ } + addr = vaddr(pages[i]->page); + pages_to_gnt[segs_to_map] = pages[i]->page; + pages[i]->persistent_gnt = NULL; +@@ -928,17 +931,18 @@ next: + } + segs_to_map = 0; + last_map = map_until; +- if (map_until != num) ++ if (!ret && map_until != num) + goto again; + +- return ret; +- +-out_of_memory: +- pr_alert("%s: out of memory\n", __func__); +- put_free_pages(ring, pages_to_gnt, segs_to_map); +- for (i = last_map; i < num; i++) ++out: ++ for (i = last_map; i < num; i++) { ++ /* Don't zap current batch's valid persistent grants. */ ++ if(i >= last_map + segs_to_map) ++ pages[i]->persistent_gnt = NULL; + pages[i]->handle = BLKBACK_INVALID_HANDLE; +- return -ENOMEM; ++ } ++ ++ return ret; + } + + static int xen_blkbk_map_seg(struct pending_req *pending_req) diff --git a/queue-4.9/xen-gntdev-correct-dev_bus_addr-handling-in-gntdev_map_grant_pages.patch b/queue-4.9/xen-gntdev-correct-dev_bus_addr-handling-in-gntdev_map_grant_pages.patch new file mode 100644 index 00000000000..f1a482be854 --- /dev/null +++ b/queue-4.9/xen-gntdev-correct-dev_bus_addr-handling-in-gntdev_map_grant_pages.patch @@ -0,0 +1,75 @@ +From dbe5283605b3bc12ca45def09cc721a0a5c853a2 Mon Sep 17 00:00:00 2001 +From: Jan Beulich +Date: Mon, 15 Feb 2021 08:51:07 +0100 +Subject: Xen/gntdev: correct dev_bus_addr handling in gntdev_map_grant_pages() + +From: Jan Beulich + +commit dbe5283605b3bc12ca45def09cc721a0a5c853a2 upstream. + +We may not skip setting the field in the unmap structure when +GNTMAP_device_map is in use - such an unmap would fail to release the +respective resources (a page ref in the hypervisor). Otoh the field +doesn't need setting at all when GNTMAP_device_map is not in use. + +To record the value for unmapping, we also better don't use our local +p2m: In particular after a subsequent change it may not have got updated +for all the batch elements. Instead it can simply be taken from the +respective map's results. + +We can additionally avoid playing this game altogether for the kernel +part of the mappings in (x86) PV mode. + +This is part of XSA-361. + +Signed-off-by: Jan Beulich +Cc: stable@vger.kernel.org +Reviewed-by: Stefano Stabellini +Signed-off-by: Juergen Gross +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/xen/gntdev.c | 16 +++++++++++++--- + 1 file changed, 13 insertions(+), 3 deletions(-) + +--- a/drivers/xen/gntdev.c ++++ b/drivers/xen/gntdev.c +@@ -293,18 +293,25 @@ static int map_grant_pages(struct grant_ + * to the kernel linear addresses of the struct pages. + * These ptes are completely different from the user ptes dealt + * with find_grant_ptes. ++ * Note that GNTMAP_device_map isn't needed here: The ++ * dev_bus_addr output field gets consumed only from ->map_ops, ++ * and by not requesting it when mapping we also avoid needing ++ * to mirror dev_bus_addr into ->unmap_ops (and holding an extra ++ * reference to the page in the hypervisor). + */ ++ unsigned int flags = (map->flags & ~GNTMAP_device_map) | ++ GNTMAP_host_map; ++ + for (i = 0; i < map->count; i++) { + unsigned long address = (unsigned long) + pfn_to_kaddr(page_to_pfn(map->pages[i])); + BUG_ON(PageHighMem(map->pages[i])); + +- gnttab_set_map_op(&map->kmap_ops[i], address, +- map->flags | GNTMAP_host_map, ++ gnttab_set_map_op(&map->kmap_ops[i], address, flags, + map->grants[i].ref, + map->grants[i].domid); + gnttab_set_unmap_op(&map->kunmap_ops[i], address, +- map->flags | GNTMAP_host_map, -1); ++ flags, -1); + } + } + +@@ -320,6 +327,9 @@ static int map_grant_pages(struct grant_ + continue; + } + ++ if (map->flags & GNTMAP_device_map) ++ map->unmap_ops[i].dev_bus_addr = map->map_ops[i].dev_bus_addr; ++ + map->unmap_ops[i].handle = map->map_ops[i].handle; + if (use_ptemod) + map->kunmap_ops[i].handle = map->kmap_ops[i].handle; diff --git a/queue-4.9/xen-gntdev-correct-error-checking-in-gntdev_map_grant_pages.patch b/queue-4.9/xen-gntdev-correct-error-checking-in-gntdev_map_grant_pages.patch new file mode 100644 index 00000000000..796fe99887f --- /dev/null +++ b/queue-4.9/xen-gntdev-correct-error-checking-in-gntdev_map_grant_pages.patch @@ -0,0 +1,77 @@ +From ebee0eab08594b2bd5db716288a4f1ae5936e9bc Mon Sep 17 00:00:00 2001 +From: Jan Beulich +Date: Mon, 15 Feb 2021 08:52:27 +0100 +Subject: Xen/gntdev: correct error checking in gntdev_map_grant_pages() + +From: Jan Beulich + +commit ebee0eab08594b2bd5db716288a4f1ae5936e9bc upstream. + +Failure of the kernel part of the mapping operation should also be +indicated as an error to the caller, or else it may assume the +respective kernel VA is okay to access. + +Furthermore gnttab_map_refs() failing still requires recording +successfully mapped handles, so they can be unmapped subsequently. This +in turn requires there to be a way to tell full hypercall failure from +partial success - preset map_op status fields such that they won't +"happen" to look as if the operation succeeded. + +Also again 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 +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/xen/gntdev.c | 17 +++++++++-------- + include/xen/grant_table.h | 1 + + 2 files changed, 10 insertions(+), 8 deletions(-) + +--- a/drivers/xen/gntdev.c ++++ b/drivers/xen/gntdev.c +@@ -318,21 +318,22 @@ static int map_grant_pages(struct grant_ + pr_debug("map %d+%d\n", map->index, map->count); + err = gnttab_map_refs(map->map_ops, use_ptemod ? map->kmap_ops : NULL, + map->pages, map->count); +- if (err) +- return err; + + for (i = 0; i < map->count; i++) { +- if (map->map_ops[i].status) { ++ if (map->map_ops[i].status == GNTST_okay) ++ map->unmap_ops[i].handle = map->map_ops[i].handle; ++ else if (!err) + err = -EINVAL; +- continue; +- } + + if (map->flags & GNTMAP_device_map) + map->unmap_ops[i].dev_bus_addr = map->map_ops[i].dev_bus_addr; + +- map->unmap_ops[i].handle = map->map_ops[i].handle; +- if (use_ptemod) +- map->kunmap_ops[i].handle = map->kmap_ops[i].handle; ++ if (use_ptemod) { ++ if (map->kmap_ops[i].status == GNTST_okay) ++ map->kunmap_ops[i].handle = map->kmap_ops[i].handle; ++ else if (!err) ++ err = -EINVAL; ++ } + } + return err; + } +--- a/include/xen/grant_table.h ++++ b/include/xen/grant_table.h +@@ -157,6 +157,7 @@ gnttab_set_map_op(struct gnttab_map_gran + map->flags = flags; + map->ref = ref; + map->dom = domid; ++ map->status = 1; /* arbitrary positive value */ + } + + static inline void diff --git a/queue-4.9/xen-netback-don-t-handle-error-by-bug.patch b/queue-4.9/xen-netback-don-t-handle-error-by-bug.patch new file mode 100644 index 00000000000..821e7cbe6f0 --- /dev/null +++ b/queue-4.9/xen-netback-don-t-handle-error-by-bug.patch @@ -0,0 +1,44 @@ +From 3194a1746e8aabe86075fd3c5e7cf1f4632d7f16 Mon Sep 17 00:00:00 2001 +From: Jan Beulich +Date: Mon, 15 Feb 2021 08:55:31 +0100 +Subject: xen-netback: don't "handle" error by BUG() + +From: Jan Beulich + +commit 3194a1746e8aabe86075fd3c5e7cf1f4632d7f16 upstream. + +In particular -ENOMEM may come back here, from set_foreign_p2m_mapping(). +Don't make problems worse, the more that handling elsewhere (together +with map's status fields now indicating whether a mapping wasn't even +attempted, and hence has to be considered failed) doesn't require this +odd way of dealing with errors. + +This is part of XSA-362. + +Signed-off-by: Jan Beulich +Cc: stable@vger.kernel.org +Reviewed-by: Juergen Gross +Signed-off-by: Juergen Gross +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/net/xen-netback/netback.c | 4 +--- + 1 file changed, 1 insertion(+), 3 deletions(-) + +--- a/drivers/net/xen-netback/netback.c ++++ b/drivers/net/xen-netback/netback.c +@@ -1328,13 +1328,11 @@ int xenvif_tx_action(struct xenvif_queue + return 0; + + gnttab_batch_copy(queue->tx_copy_ops, nr_cops); +- if (nr_mops != 0) { ++ if (nr_mops != 0) + ret = gnttab_map_refs(queue->tx_map_ops, + NULL, + queue->pages_to_map, + nr_mops); +- BUG_ON(ret); +- } + + work_done = xenvif_tx_submit(queue); + diff --git a/queue-4.9/xen-scsiback-don-t-handle-error-by-bug.patch b/queue-4.9/xen-scsiback-don-t-handle-error-by-bug.patch new file mode 100644 index 00000000000..1f4fad5001c --- /dev/null +++ b/queue-4.9/xen-scsiback-don-t-handle-error-by-bug.patch @@ -0,0 +1,44 @@ +From 7c77474b2d22176d2bfb592ec74e0f2cb71352c9 Mon Sep 17 00:00:00 2001 +From: Jan Beulich +Date: Mon, 15 Feb 2021 08:55:57 +0100 +Subject: xen-scsiback: don't "handle" error by BUG() + +From: Jan Beulich + +commit 7c77474b2d22176d2bfb592ec74e0f2cb71352c9 upstream. + +In particular -ENOMEM may come back here, from set_foreign_p2m_mapping(). +Don't make problems worse, the more that handling elsewhere (together +with map's status fields now indicating whether a mapping wasn't even +attempted, and hence has to be considered failed) doesn't require this +odd way of dealing with errors. + +This is part of XSA-362. + +Signed-off-by: Jan Beulich +Cc: stable@vger.kernel.org +Reviewed-by: Juergen Gross +Signed-off-by: Juergen Gross +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/xen/xen-scsiback.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +--- a/drivers/xen/xen-scsiback.c ++++ b/drivers/xen/xen-scsiback.c +@@ -423,12 +423,12 @@ static int scsiback_gnttab_data_map_batc + return 0; + + err = gnttab_map_refs(map, NULL, pg, cnt); +- BUG_ON(err); + for (i = 0; i < cnt; i++) { + if (unlikely(map[i].status != GNTST_okay)) { + pr_err("invalid buffer -- could not remap it\n"); + map[i].handle = SCSIBACK_INVALID_HANDLE; +- err = -ENOMEM; ++ if (!err) ++ err = -ENOMEM; + } else { + get_page(pg[i]); + } diff --git a/queue-4.9/xen-x86-also-check-kernel-mapping-in-set_foreign_p2m_mapping.patch b/queue-4.9/xen-x86-also-check-kernel-mapping-in-set_foreign_p2m_mapping.patch new file mode 100644 index 00000000000..22f57e8f8a7 --- /dev/null +++ b/queue-4.9/xen-x86-also-check-kernel-mapping-in-set_foreign_p2m_mapping.patch @@ -0,0 +1,38 @@ +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() + +From: Jan Beulich + +commit b512e1b077e5ccdbd6e225b15d934ab12453b70a upstream. + +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 +Signed-off-by: Greg Kroah-Hartman + +--- + arch/x86/xen/p2m.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +--- a/arch/x86/xen/p2m.c ++++ b/arch/x86/xen/p2m.c +@@ -725,7 +725,8 @@ int set_foreign_p2m_mapping(struct gntta + 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) { diff --git a/queue-4.9/xen-x86-don-t-bail-early-from-clear_foreign_p2m_mapping.patch b/queue-4.9/xen-x86-don-t-bail-early-from-clear_foreign_p2m_mapping.patch new file mode 100644 index 00000000000..4fa92361527 --- /dev/null +++ b/queue-4.9/xen-x86-don-t-bail-early-from-clear_foreign_p2m_mapping.patch @@ -0,0 +1,59 @@ +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() + +From: Jan Beulich + +commit a35f2ef3b7376bfd0a57f7844bd7454389aae1fc upstream. + +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 +Signed-off-by: Greg Kroah-Hartman + +--- + arch/x86/xen/p2m.c | 12 +++++------- + 1 file changed, 5 insertions(+), 7 deletions(-) + +--- a/arch/x86/xen/p2m.c ++++ b/arch/x86/xen/p2m.c +@@ -763,17 +763,15 @@ int clear_foreign_p2m_mapping(struct gnt + 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); -- 2.47.2