[PATCH] xsm/dummy: harden against speculative abuse

[PATCH] xsm/dummy: harden against speculative abuse

[Jan Beulich]

First of all don't open-code is_control_domain(), which is already
suitably using evaluate_nospec(). Then also apply this construct to the
other paths of xsm_default_action(). Also guard two paths not using this
function.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
While the functions are always_inline I'm not entirely certain we can
get away with doing this inside of them, rather than in the callers. It
will certainly take more to also guard builds with non-dummy XSM.

--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -76,20 +76,20 @@ static always_inline int xsm_default_act
     case XSM_HOOK:
         return 0;
     case XSM_TARGET:
-        if ( src == target )
+        if ( evaluate_nospec(src == target) )
         {
             return 0;
     case XSM_XS_PRIV:
-            if ( is_xenstore_domain(src) )
+            if ( evaluate_nospec(is_xenstore_domain(src)) )
                 return 0;
         }
         /* fall through */
     case XSM_DM_PRIV:
-        if ( target && src->target == target )
+        if ( target && evaluate_nospec(src->target == target) )
             return 0;
         /* fall through */
     case XSM_PRIV:
-        if ( src->is_privileged )
+        if ( !is_control_domain(src) )
             return 0;
         return -EPERM;
     default:
@@ -656,7 +656,7 @@ static XSM_INLINE int xsm_mmu_update(XSM
     XSM_ASSERT_ACTION(XSM_TARGET);
     if ( f != dom_io )
         rc = xsm_default_action(action, d, f);
-    if ( t && !rc )
+    if ( evaluate_nospec(t) && !rc )
         rc = xsm_default_action(action, d, t);
     return rc;
 }
@@ -750,6 +750,7 @@ static XSM_INLINE int xsm_xen_version (X
     case XENVER_platform_parameters:
     case XENVER_get_features:
         /* These sub-ops ignore the permission checks and return data. */
+        block_speculation();
         return 0;
     case XENVER_extraversion:
     case XENVER_compile_info:

Re: [PATCH] xsm/dummy: harden against speculative abuse

[Jan Beulich]

On 17.12.2020 12:57, Jan Beulich wrote:
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -76,20 +76,20 @@ static always_inline int xsm_default_act
>      case XSM_HOOK:
>          return 0;
>      case XSM_TARGET:
> -        if ( src == target )
> +        if ( evaluate_nospec(src == target) )
>          {
>              return 0;
>      case XSM_XS_PRIV:
> -            if ( is_xenstore_domain(src) )
> +            if ( evaluate_nospec(is_xenstore_domain(src)) )
>                  return 0;
>          }
>          /* fall through */
>      case XSM_DM_PRIV:
> -        if ( target && src->target == target )
> +        if ( target && evaluate_nospec(src->target == target) )
>              return 0;
>          /* fall through */
>      case XSM_PRIV:
> -        if ( src->is_privileged )
> +        if ( !is_control_domain(src) )
>              return 0;
>          return -EPERM;

And a stray ! slipped in here. Now fixed.

Jan

Re: [PATCH] xsm/dummy: harden against speculative abuse

[Wei Liu]

On Mon, Dec 21, 2020 at 02:15:55PM +0100, Jan Beulich wrote:
> On 17.12.2020 12:57, Jan Beulich wrote:
> > --- a/xen/include/xsm/dummy.h
> > +++ b/xen/include/xsm/dummy.h
> > @@ -76,20 +76,20 @@ static always_inline int xsm_default_act
> >      case XSM_HOOK:
> >          return 0;
> >      case XSM_TARGET:
> > -        if ( src == target )
> > +        if ( evaluate_nospec(src == target) )
> >          {
> >              return 0;
> >      case XSM_XS_PRIV:
> > -            if ( is_xenstore_domain(src) )
> > +            if ( evaluate_nospec(is_xenstore_domain(src)) )
> >                  return 0;
> >          }
> >          /* fall through */
> >      case XSM_DM_PRIV:
> > -        if ( target && src->target == target )
> > +        if ( target && evaluate_nospec(src->target == target) )
> >              return 0;
> >          /* fall through */
> >      case XSM_PRIV:
> > -        if ( src->is_privileged )
> > +        if ( !is_control_domain(src) )
> >              return 0;
> >          return -EPERM;
> 
> And a stray ! slipped in here. Now fixed.

FWIW:

Reviewed-by: Wei Liu <wl@xen.org>

> 
> Jan