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

[Julien Grall <julien.grall@citrix.com>]

Hi Ian,

On 03/21/2014 10:44 AM, Ian Campbell wrote:
> 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?

We have to check somewhere that the removed gfn corresponding to the mfn.
Otherwise the toolstack may be able to remove any page as long as the
MFN is in the iomem permitted range.

I think this is the best approach to check it.

Regards,

-- 
Julien Grall