Re: [Xen-devel] [PATCH MM-PART3 v2 11/12] xen/arm: mm: Don't open-code Xen PT update in {set, clear}_fixmap()

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 6/12/19 11:33 PM, Stefano Stabellini wrote:
> On Tue, 14 May 2019, Julien Grall wrote:
> > {set, clear}_fixmap() are currently open-coding update to the Xen
> > page-tables. This can be avoided by using the generic helpers
> > map_pages_to_xen() and destroy_xen_mappings().
> >
> > Both function are not meant to fail for fixmap, hence the BUG_ON()
> > checking the return.
> BUG_ON crashes the hypervisor even in non-DEBUG builds. Would an ASSERT
> be a better choice?
The ASSERT() would disappear in non-debug potentially leading to unknown 
consequence.
If we imagine that map_pages_to_xen() fails, then it likely means that 
mapping has not been done/removed.
As set_fixmap() does not return an error, this means that the user may 
try to access an invalid mapping and therefore crash the hypervisor.
As clear_fixmap() does not return an error, this means that subsequent 
set_fixmap() may fail because map_pages_to_xen() does not allow to 
replace valid mapping.
Ideally we would want to propagate the error, however all the call to 
the functions happen during boot. So most likely the user will 
panic/BUG_ON as you this hint something has gone really wrong and we 
don't want to continue further.
Cheers,

--
Julien Grall

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