Re: [Xen-devel] [PATCH v3 2/5] arch, arm: add consistency checks to REMOVE p2m changes

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Sat, 2014-03-15 at 21:11 +0100, Arianna Avanzini wrote:
> Currently, the REMOVE case of the switch in apply_p2m_changes()
> does not perform any consistency check on the mapping to be removed.
> More in detail, the code does not check that the type of the entry
> is correct in case of I/O memory mapping removal; also, the code
> does not check if the guest address to be unmapped is actually mapped
> to the machine address given as a parameter.
> This commit attempts to add the above-described consistency checks
> to the REMOVE path of apply_p2m_changes(). This is instrumental to
> the following commit which implements the possibility to trigger
> the removal of p2m ranges via the memory_mapping DOMCTL for ARM.

I'm not sure I follow why this is needed, is there some reason
apply_p2m_changes(REMOVE, ...) should not just remove whatever it is
asked to? What is the downside if the memory_mapping domctl removes
something which is not a memory mapping?

If it's just "a bug" then I think the toolstack should "Not Do That
Then". If the bug might have security implications then perhaps we need
to worry about it, but do you have such a case in mind?

> +                    /*
> +                     * Ensure that, if we are trying to unmap I/O memory
> +                     * ranges, the given gfn is p2m_mmio_direct.
> +                     */
> +                    if ( t == p2m_mmio_direct ? _t != p2m_mmio_direct : 0 ||

If we really do need this sort of behaviour (see above) then I think
this check should be made more generic: 
        if ( t != p2m_invalid && pte.p2m.type != t )
                an error

p2m_invalid is a placeholder in this api for "don't care", since it
doesn't make sense to worry about removing a p2m which doesn't map
anything.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel