virt_to_maddr() is using the hardware page-table walk instructions to
translate a virtual address to physical address. The function should
only be called on virtual address mapped.
_end points past the end of Xen binary and may not be mapped when the
binary size is page-aligned. This means virt_to_maddr() will not be able
to do the translation and therefore crash Xen.
Note there is also an off-by-one issue in this code, but the panic will
trump that.
Both issues can be fixed by using _end - 1 in the check.
Signed-off-by: Julien Grall <julien.grall@arm.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien@xen.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wl@xen.org>
x86 seems to be affected by the off-by-one issue. Jan, Andrew?
This could be reached by a domain via XEN_SYSCTL_page_offline_op.
However, the operation is not security supported (see XSA-77). So we are
fine here.
Changes in v2:
- Cast _end to vaddr_t to prevent UB.
- Add Juergen's released-acked-by
---
xen/include/asm-arm/mm.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index 262d92f18d..333efd3a60 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -153,7 +153,7 @@ extern unsigned long xenheap_base_pdx;
#define is_xen_fixed_mfn(mfn) \
((mfn_to_maddr(mfn) >= virt_to_maddr(&_start)) && \
- (mfn_to_maddr(mfn) <= virt_to_maddr(&_end)))
+ (mfn_to_maddr(mfn) <= virt_to_maddr((vaddr_t)_end - 1)))
#define page_get_owner(_p) (_p)->v.inuse.domain
#define page_set_owner(_p,_d) ((_p)->v.inuse.domain = (_d))
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On 16.10.2019 12:53, Julien Grall wrote: > virt_to_maddr() is using the hardware page-table walk instructions to > translate a virtual address to physical address. The function should > only be called on virtual address mapped. > > _end points past the end of Xen binary and may not be mapped when the > binary size is page-aligned. This means virt_to_maddr() will not be able > to do the translation and therefore crash Xen. > > Note there is also an off-by-one issue in this code, but the panic will > trump that. > > Both issues can be fixed by using _end - 1 in the check. > > Signed-off-by: Julien Grall <julien.grall@arm.com> > Release-acked-by: Juergen Gross <jgross@suse.com> > > --- > > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > Cc: George Dunlap <George.Dunlap@eu.citrix.com> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Julien Grall <julien@xen.org> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Tim Deegan <tim@xen.org> > Cc: Wei Liu <wl@xen.org> > > x86 seems to be affected by the off-by-one issue. Jan, Andrew? Indeed. I'm in the process of making a patch. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On Wed, 16 Oct 2019, Julien Grall wrote: > virt_to_maddr() is using the hardware page-table walk instructions to > translate a virtual address to physical address. The function should > only be called on virtual address mapped. > > _end points past the end of Xen binary and may not be mapped when the > binary size is page-aligned. This means virt_to_maddr() will not be able > to do the translation and therefore crash Xen. > > Note there is also an off-by-one issue in this code, but the panic will > trump that. > > Both issues can be fixed by using _end - 1 in the check. > > Signed-off-by: Julien Grall <julien.grall@arm.com> > Release-acked-by: Juergen Gross <jgross@suse.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > Cc: George Dunlap <George.Dunlap@eu.citrix.com> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Julien Grall <julien@xen.org> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Tim Deegan <tim@xen.org> > Cc: Wei Liu <wl@xen.org> > > x86 seems to be affected by the off-by-one issue. Jan, Andrew? > > This could be reached by a domain via XEN_SYSCTL_page_offline_op. > However, the operation is not security supported (see XSA-77). So we are > fine here. > > Changes in v2: > - Cast _end to vaddr_t to prevent UB. > - Add Juergen's released-acked-by > --- > xen/include/asm-arm/mm.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h > index 262d92f18d..333efd3a60 100644 > --- a/xen/include/asm-arm/mm.h > +++ b/xen/include/asm-arm/mm.h > @@ -153,7 +153,7 @@ extern unsigned long xenheap_base_pdx; > > #define is_xen_fixed_mfn(mfn) \ > ((mfn_to_maddr(mfn) >= virt_to_maddr(&_start)) && \ > - (mfn_to_maddr(mfn) <= virt_to_maddr(&_end))) > + (mfn_to_maddr(mfn) <= virt_to_maddr((vaddr_t)_end - 1))) > > #define page_get_owner(_p) (_p)->v.inuse.domain > #define page_set_owner(_p,_d) ((_p)->v.inuse.domain = (_d)) > -- > 2.11.0 > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
© 2016 - 2024 Red Hat, Inc.