Re: [Xen-devel] [PATCH v2] Merge IS_PRIV checks into XSM hooks

[Keir Fraser <keir@xxxxxxx>]

On 10/09/2012 22:10, "Daniel De Graaf" <dgdegra@xxxxxxxxxxxxx> wrote:

> On 09/10/2012 04:51 PM, Keir Fraser wrote:
>> On 10/09/2012 20:48, "Daniel De Graaf" <dgdegra@xxxxxxxxxxxxx> wrote:
>> 
>>> Overall, this series should not change the behavior of Xen when XSM is
>>> not enabled; however, in some cases, the exact errors that are returned
>>> will be different because security checks have been moved below validity
>>> checks. Also, once applied, newly introduced domctls and sysctls will
>>> not automatically be guarded by IS_PRIV checks - they will need to add
>>> their own permission checking code.
>> 
>> How do we guard against accidentally forgetting to do this?
> 
> The same way you guard against it when adding a new hypercall: when adding
> new functionality that needs access checks, also add the access checks.

So... We just shouldn't accidentally forget. That will work well. ;)
Historically XSM has not been top of many committers' checklists.

 -- Keir

>>> The ARM architecture is not touched at all in these patches. The only
>>> obvious breakage that I can see is due to rcu_lock_target_domain_by_id
>>> being removed, but XSM hooks will be needed for domctls and sysctls.
>> 
>> So ARM build is broken? And/or ARM is made insecure because of unchecked
>> sysctls/domctls?
>> 
>>  -- Keir
> 
> The ARM build is broken by patch #19 in this series; fixing it is fairly
> simple (I'll send a non-compile-tested version as 21/20), or you could
> postpone that patch as it's just cleanup.
> 
> Since ARM doesn't have any arch-specific domctls or sysctls yet, they are
> not insecure. You could also add an IS_PRIV check at the top of ARM's
> arch_do_{dom,sys}ctl functions if you don't want to add XSM hooks for each
> operation as in x86.
> 
>> 
>>> The rcu_lock_target_domain_by_id and rcu_lock_remote_target_domain_by_id
>>> functions are removed by this series because they act as wrappers around
>>> IS_PRIV_FOR; their callers have been changed to use XSM checks instead.
>>> 
>>> Miscellaneous updates to FLASK:
>>>     [PATCH 01/20] xsm/flask: remove inherited class attributes
>>>     [PATCH 02/20] xsm/flask: remove unneeded create_sid field
>>>     [PATCH 03/20] xen: Add versions of rcu_lock_*_domain without IS_PRIV
>>>     [PATCH 04/20] xsm/flask: add domain relabel support
>>>     [PATCH 05/20] libxl: introduce XSM relabel on build
>>>     [PATCH 06/20] flask/policy: Add domain relabel example
>>> 
>>> Preparatory new hooks:
>>>     [PATCH 07/20] arch/x86: add distinct XSM hooks for map/unmap
>>>     [PATCH 08/20] arch/x86: add missing XSM checks to XENPF_ commands
>>>     [PATCH 09/20] xsm/flask: Add checks on the domain performing the
>>> 
>>> Refactoring:
>>>     [PATCH 10/20] xsm: Add IS_PRIV checks to dummy XSM module
>>>     [PATCH 11/20] xen: use XSM instead of IS_PRIV where duplicated
>>>     [PATCH 12/20] xen: avoid calling rcu_lock_*target_domain when an XSM
>>> 
>>> Remaining IS_PRIV calls:
>>>     [PATCH 13/20] arch/x86: Add missing domctl and mem_sharing XSM hooks
>>>     [PATCH 14/20] tmem: Add access control check
>>>     [PATCH 17/20] arch/x86: use XSM hooks for get_pg_owner access checks
>>>     [PATCH 18/20] xen: Add XSM hook for XENMEM_exchange
>>> 
>>> Cleanup, FLASK updates to support IS_PRIV emulation:
>>>     [PATCH 15/20] xsm: remove unneeded xsm_call macro
>>>     [PATCH 16/20] xsm/flask: add distinct SIDs for self/target access
>>>     [PATCH 19/20] xen: remove rcu_lock_{remote_,}target_domain_by_id
>>>     [PATCH 20/20] flask: add missing operations
>>> 



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