[PATCH v4] x86/setup: disallow any src/dst overlaps when relocating Xen image

[PATCH v4] x86/setup: disallow any src/dst overlaps when relocating Xen image

[Daniel Kiper]

Commit 0d31d16 (x86/setup: do not relocate Xen over current Xen image
placement) disallowed src/dst images overlaps when relocating Xen image.
Though it deliberately allowed destination region between __image_base__
and (__image_base__ + XEN_IMG_OFFSET) overlaps with the end of source
image. And here is the problem. If anything between __page_tables_start
and __page_tables_end in source image lands in the overlap then some or
even all page table entries may not be updated. This usually means boom
in early boot which will be difficult to the investigate. So, I think
that we have three choices to fix the issue:
  - drop XEN_IMG_OFFSET from
    if ( (end > s) && (end - reloc_size + XEN_IMG_OFFSET >= __pa(_end)) )
  - add XEN_IMG_OFFSET to xen_phys_start in PFN_DOWN(xen_phys_start)
    used in loops as one of conditions,
  - change PFN_DOWN(xen_phys_start) to PFN_DOWN(xen_remap_end_pfn)
    proposed in earlier version of this patch.

This patch implements the first option. This way we will avoid all kinds
of overlaps which are always full can of worms.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 xen/arch/x86/setup.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index c0b97a7..b3a66f4 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1003,7 +1003,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
          * Is the region size greater than zero and does it begin
          * at or above the end of current Xen image placement?
          */
-        if ( (end > s) && (end - reloc_size + XEN_IMG_OFFSET >= __pa(_end)) )
+        if ( (end > s) && (end - reloc_size >= __pa(_end)) )
         {
             l4_pgentry_t *pl4e;
             l3_pgentry_t *pl3e;
-- 
1.7.10.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4] x86/setup: disallow any src/dst overlaps when relocating Xen image

[Jan Beulich]

>>> On 11.04.18 at 10:54, <daniel.kiper@oracle.com> wrote:
> On Tue, Apr 03, 2018 at 05:54:14PM +0200, Daniel Kiper wrote:
>> Commit 0d31d16 (x86/setup: do not relocate Xen over current Xen image
>> placement) disallowed src/dst images overlaps when relocating Xen image.
>> Though it deliberately allowed destination region between __image_base__
>> and (__image_base__ + XEN_IMG_OFFSET) overlaps with the end of source
>> image. And here is the problem. If anything between __page_tables_start
>> and __page_tables_end in source image lands in the overlap then some or
>> even all page table entries may not be updated. This usually means boom
>> in early boot which will be difficult to the investigate. So, I think
>> that we have three choices to fix the issue:
>>   - drop XEN_IMG_OFFSET from
>>     if ( (end > s) && (end - reloc_size + XEN_IMG_OFFSET >= __pa(_end)) )
>>   - add XEN_IMG_OFFSET to xen_phys_start in PFN_DOWN(xen_phys_start)
>>     used in loops as one of conditions,
>>   - change PFN_DOWN(xen_phys_start) to PFN_DOWN(xen_remap_end_pfn)
>>     proposed in earlier version of this patch.
>>
>> This patch implements the first option. This way we will avoid all kinds
>> of overlaps which are always full can of worms.
>>
>> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
> 
> Ping?

I'll get to it, but it has missed 4.11 anyway, so I don't think there's
an urgency here.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4] x86/setup: disallow any src/dst overlaps when relocating Xen image

[Jan Beulich]

>>> On 03.04.18 at 17:54, <daniel.kiper@oracle.com> wrote:
> Commit 0d31d16 (x86/setup: do not relocate Xen over current Xen image
> placement) disallowed src/dst images overlaps when relocating Xen image.
> Though it deliberately allowed destination region between __image_base__
> and (__image_base__ + XEN_IMG_OFFSET) overlaps with the end of source
> image. And here is the problem. If anything between __page_tables_start
> and __page_tables_end in source image lands in the overlap then some or
> even all page table entries may not be updated. This usually means boom
> in early boot which will be difficult to the investigate. So, I think
> that we have three choices to fix the issue:
>   - drop XEN_IMG_OFFSET from
>     if ( (end > s) && (end - reloc_size + XEN_IMG_OFFSET >= __pa(_end)) )
>   - add XEN_IMG_OFFSET to xen_phys_start in PFN_DOWN(xen_phys_start)
>     used in loops as one of conditions,
>   - change PFN_DOWN(xen_phys_start) to PFN_DOWN(xen_remap_end_pfn)
>     proposed in earlier version of this patch.
> 
> This patch implements the first option. This way we will avoid all kinds
> of overlaps which are always full can of worms.

Personally I'd like option 2 better, as there's nothing of interest in the
[0,XEN_IMG_OFFSET) range. Instead of modifying every
PFN_DOWN(xen_phys_start), perhaps simply introduce a local variable,
accompanied by a suitable comment.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel