[Xen-devel] [PATCH for-4.11 0/2] XSA-295 backport adjustments

[Xen-devel] [PATCH for-4.11 0/2] XSA-295 backport adjustments

[Jan Beulich]

XSM was broken (patch 1) and the removal of arch_evtchn_inject() was
only partially done (patch 2).

1: XSM: adjust Kconfig names
2: x86: drop arch_evtchn_inject()

I'm not going to wait for any acks; in case of issues further incremental
fixups will need to do.

These adjustments will also be needed on older branches, but I'd first
like to see how 4.11 goes (not the least to get 4.11.2 out the door
_finally_).

Jan



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

[Xen-devel] [PATCH for-4.11 1/2] XSM: adjust Kconfig names

[Jan Beulich]

Since the Kconfig option renaming was not backported, the new uses of
involved CONFIG_* settings should have been adopted to the existing
names in the XSA-295 series. Do this now, also changing XSM_SILO to just
SILO to better match its FLASK counterpart.

To avoid breaking the Kconfig menu structure also adjust XSM_POLICY's
dependency (as was also silently done on master during the renaming).

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -130,7 +130,7 @@ config FLASK_AVC_STATS
 config XSM_POLICY
 	bool "Compile Xen with a built-in security policy"
 	default y if HAS_CHECKPOLICY = "y"
-	depends on XSM
+	depends on FLASK
 	---help---
 	  This includes a default XSM policy in the hypervisor so that the
 	  bootloader does not need to load a policy to get sane behavior from an
@@ -143,7 +143,7 @@ config XSM_POLICY
 
 	  If unsure, say Y.
 
-config XSM_SILO
+config SILO
 	def_bool y
 	prompt "SILO support"
 	depends on XSM
@@ -158,16 +158,16 @@ config XSM_SILO
 choice
 	prompt "Default XSM implementation"
 	depends on XSM
-	default XSM_SILO_DEFAULT if XSM_SILO && ARM
-	default XSM_FLASK_DEFAULT if XSM_FLASK
-	default XSM_SILO_DEFAULT if XSM_SILO
+	default XSM_SILO_DEFAULT if SILO && ARM
+	default XSM_FLASK_DEFAULT if FLASK
+	default XSM_SILO_DEFAULT if SILO
 	default XSM_DUMMY_DEFAULT
 	config XSM_DUMMY_DEFAULT
 		bool "Match non-XSM behavior"
 	config XSM_FLASK_DEFAULT
-		bool "FLux Advanced Security Kernel" if XSM_FLASK
+		bool "FLux Advanced Security Kernel" if FLASK
 	config XSM_SILO_DEFAULT
-		bool "SILO" if XSM_SILO
+		bool "SILO" if SILO
 endchoice
 
 config LATE_HWDOM
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -738,7 +738,7 @@ extern const unsigned char xsm_init_poli
 extern const unsigned int xsm_init_policy_size;
 #endif
 
-#ifdef CONFIG_XSM_SILO
+#ifdef CONFIG_SILO
 extern void silo_init(void);
 #else
 static inline void silo_init(void) {}
--- a/xen/xsm/Makefile
+++ b/xen/xsm/Makefile
@@ -1,6 +1,6 @@
 obj-y += xsm_core.o
 obj-$(CONFIG_XSM) += xsm_policy.o
 obj-$(CONFIG_XSM) += dummy.o
-obj-$(CONFIG_XSM_SILO) += silo.o
+obj-$(CONFIG_SILO) += silo.o
 
 subdir-$(CONFIG_FLASK) += flask
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -38,9 +38,9 @@ enum xsm_bootparam {
 };
 
 static enum xsm_bootparam __initdata xsm_bootparam =
-#ifdef CONFIG_XSM_FLASK_DEFAULT
+#if defined(CONFIG_XSM_FLASK_DEFAULT)
     XSM_BOOTPARAM_FLASK;
-#elif CONFIG_XSM_SILO_DEFAULT
+#elif defined(CONFIG_XSM_SILO_DEFAULT)
     XSM_BOOTPARAM_SILO;
 #else
     XSM_BOOTPARAM_DUMMY;
@@ -52,11 +52,11 @@ static int __init parse_xsm_param(const
 
     if ( !strcmp(s, "dummy") )
         xsm_bootparam = XSM_BOOTPARAM_DUMMY;
-#ifdef CONFIG_XSM_FLASK
+#ifdef CONFIG_FLASK
     else if ( !strcmp(s, "flask") )
         xsm_bootparam = XSM_BOOTPARAM_FLASK;
 #endif
-#ifdef CONFIG_XSM_SILO
+#ifdef CONFIG_SILO
     else if ( !strcmp(s, "silo") )
         xsm_bootparam = XSM_BOOTPARAM_SILO;
 #endif





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

[Xen-devel] [PATCH for-4.11 2/2] x86: drop arch_evtchn_inject()

[Jan Beulich]

For whatever reason this was omitted from the backport of d9195962a6
("events: drop arch_evtchn_inject()").

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/hvm/irq.c
+++ b/xen/arch/x86/hvm/irq.c
@@ -562,12 +562,6 @@ int hvm_local_events_need_delivery(struc
     return !hvm_interrupt_blocked(v, intack);
 }
 
-void arch_evtchn_inject(struct vcpu *v)
-{
-    if ( is_hvm_vcpu(v) )
-        hvm_assert_evtchn_irq(v);
-}
-
 static void irq_dump(struct domain *d)
 {
     struct hvm_irq *hvm_irq = hvm_domain_irq(d);



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

Re: [Xen-devel] [PATCH for-4.11 2/2] x86: drop arch_evtchn_inject()

[Andrew Cooper]

On 18/06/2019 15:04, Jan Beulich wrote:
> For whatever reason this was omitted from the backport of d9195962a6
> ("events: drop arch_evtchn_inject()").
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [Xen-devel] [PATCH for-4.11 1/2] XSM: adjust Kconfig names

[Julien Grall]

Hi Jan,

On 18/06/2019 15:04, Jan Beulich wrote:
> Since the Kconfig option renaming was not backported, the new uses of
> involved CONFIG_* settings should have been adopted to the existing
> names in the XSA-295 series. Do this now, also changing XSM_SILO to just
> SILO to better match its FLASK counterpart.
> 
> To avoid breaking the Kconfig menu structure also adjust XSM_POLICY's
> dependency (as was also silently done on master during the renaming).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Sorry for the breakage. To avoid such blunder during XSAs, would it be possible 
to test them on osstest before they are published?

Also, do we need to update the advisory?

Acked-by: Julien Grall <julien.grall@arm.com>

> 
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -130,7 +130,7 @@ config FLASK_AVC_STATS
>   config XSM_POLICY
>   	bool "Compile Xen with a built-in security policy"
>   	default y if HAS_CHECKPOLICY = "y"
> -	depends on XSM
> +	depends on FLASK
>   	---help---
>   	  This includes a default XSM policy in the hypervisor so that the
>   	  bootloader does not need to load a policy to get sane behavior from an
> @@ -143,7 +143,7 @@ config XSM_POLICY
>   
>   	  If unsure, say Y.
>   
> -config XSM_SILO
> +config SILO
>   	def_bool y
>   	prompt "SILO support"
>   	depends on XSM
> @@ -158,16 +158,16 @@ config XSM_SILO
>   choice
>   	prompt "Default XSM implementation"
>   	depends on XSM
> -	default XSM_SILO_DEFAULT if XSM_SILO && ARM
> -	default XSM_FLASK_DEFAULT if XSM_FLASK
> -	default XSM_SILO_DEFAULT if XSM_SILO
> +	default XSM_SILO_DEFAULT if SILO && ARM
> +	default XSM_FLASK_DEFAULT if FLASK
> +	default XSM_SILO_DEFAULT if SILO
>   	default XSM_DUMMY_DEFAULT
>   	config XSM_DUMMY_DEFAULT
>   		bool "Match non-XSM behavior"
>   	config XSM_FLASK_DEFAULT
> -		bool "FLux Advanced Security Kernel" if XSM_FLASK
> +		bool "FLux Advanced Security Kernel" if FLASK
>   	config XSM_SILO_DEFAULT
> -		bool "SILO" if XSM_SILO
> +		bool "SILO" if SILO
>   endchoice
>   
>   config LATE_HWDOM
> --- a/xen/include/xsm/xsm.h
> +++ b/xen/include/xsm/xsm.h
> @@ -738,7 +738,7 @@ extern const unsigned char xsm_init_poli
>   extern const unsigned int xsm_init_policy_size;
>   #endif
>   
> -#ifdef CONFIG_XSM_SILO
> +#ifdef CONFIG_SILO
>   extern void silo_init(void);
>   #else
>   static inline void silo_init(void) {}
> --- a/xen/xsm/Makefile
> +++ b/xen/xsm/Makefile
> @@ -1,6 +1,6 @@
>   obj-y += xsm_core.o
>   obj-$(CONFIG_XSM) += xsm_policy.o
>   obj-$(CONFIG_XSM) += dummy.o
> -obj-$(CONFIG_XSM_SILO) += silo.o
> +obj-$(CONFIG_SILO) += silo.o
>   
>   subdir-$(CONFIG_FLASK) += flask
> --- a/xen/xsm/xsm_core.c
> +++ b/xen/xsm/xsm_core.c
> @@ -38,9 +38,9 @@ enum xsm_bootparam {
>   };
>   
>   static enum xsm_bootparam __initdata xsm_bootparam =
> -#ifdef CONFIG_XSM_FLASK_DEFAULT
> +#if defined(CONFIG_XSM_FLASK_DEFAULT)
>       XSM_BOOTPARAM_FLASK;
> -#elif CONFIG_XSM_SILO_DEFAULT
> +#elif defined(CONFIG_XSM_SILO_DEFAULT)
>       XSM_BOOTPARAM_SILO;
>   #else
>       XSM_BOOTPARAM_DUMMY;
> @@ -52,11 +52,11 @@ static int __init parse_xsm_param(const
>   
>       if ( !strcmp(s, "dummy") )
>           xsm_bootparam = XSM_BOOTPARAM_DUMMY;
> -#ifdef CONFIG_XSM_FLASK
> +#ifdef CONFIG_FLASK
>       else if ( !strcmp(s, "flask") )
>           xsm_bootparam = XSM_BOOTPARAM_FLASK;
>   #endif
> -#ifdef CONFIG_XSM_SILO
> +#ifdef CONFIG_SILO
>       else if ( !strcmp(s, "silo") )
>           xsm_bootparam = XSM_BOOTPARAM_SILO;
>   #endif
> 
> 
> 
> 

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH for-4.11 1/2] XSM: adjust Kconfig names

[Jan Beulich]

>>> On 18.06.19 at 16:11, <julien.grall@arm.com> wrote:
> On 18/06/2019 15:04, Jan Beulich wrote:
>> Since the Kconfig option renaming was not backported, the new uses of
>> involved CONFIG_* settings should have been adopted to the existing
>> names in the XSA-295 series. Do this now, also changing XSM_SILO to just
>> SILO to better match its FLASK counterpart.
>> 
>> To avoid breaking the Kconfig menu structure also adjust XSM_POLICY's
>> dependency (as was also silently done on master during the renaming).
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Sorry for the breakage. To avoid such blunder during XSAs, would it be possible 
> to test them on osstest before they are published?

That's an option, but would cause further delays. How exactly to
arrange for this I'm the wrong one to ask, though.

But let's face it: The patch changing Kconfig not having applied
without fuzz should have told whoever did the backport to look
more closely.

What I'd like to ask for in the future in any case though is that after
pushing stuff to stable trees you would please check the osstest
reports, and in case of regressions invest at least some time into
figuring out what broke. Right now, even with the XSM tests
(hopefully) taken care of there's still a flood of armhf failures, which
may or may not be due to environmental issues.

> Also, do we need to update the advisory?

Dunno. I didn't do full analysis of what may go wrong, I've just worked
my way far enough to understand what needs fixing. Whether an
update is needed imo largely depends on whether the purpose of the
patches wasn't fulfilled. People actually using XSM will notice very
quickly that things don't work anymore, as can be seen from the
osstest cases.

> Acked-by: Julien Grall <julien.grall@arm.com>

Thanks, Jan



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

Re: [Xen-devel] [PATCH for-4.11 1/2] XSM: adjust Kconfig names

[Julien Grall]

(+ Ian)

On 18/06/2019 15:26, Jan Beulich wrote:
>>>> On 18.06.19 at 16:11, <julien.grall@arm.com> wrote:
>> On 18/06/2019 15:04, Jan Beulich wrote:
>>> Since the Kconfig option renaming was not backported, the new uses of
>>> involved CONFIG_* settings should have been adopted to the existing
>>> names in the XSA-295 series. Do this now, also changing XSM_SILO to just
>>> SILO to better match its FLASK counterpart.
>>>
>>> To avoid breaking the Kconfig menu structure also adjust XSM_POLICY's
>>> dependency (as was also silently done on master during the renaming).
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> Sorry for the breakage. To avoid such blunder during XSAs, would it be possible
>> to test them on osstest before they are published?
> 
> That's an option, but would cause further delays. How exactly to
> arrange for this I'm the wrong one to ask, though.

Indeed, however testings need to be done manually at the moment. With 6 trees to 
take care, this is more likely going to delay more than automatic testing.

Anyway, that's only a suggestion to improve XSA testings (at least on Arm). :)

> 
> But let's face it: The patch changing Kconfig not having applied
> without fuzz should have told whoever did the backport to look
> more closely.
> 
> What I'd like to ask for in the future in any case though is that after
> pushing stuff to stable trees you would please check the osstest
> reports, and in case of regressions invest at least some time into
> figuring out what broke. Right now, even with the XSM tests
> (hopefully) taken care of there's still a flood of armhf failures, which
> may or may not be due to environmental issues.

I usually look over osstest but fail to detect this was an issue because of the 
XSAs. Regarding the other armhf failure, Ian already pointed out on IRC.

However, I will not have time to look at it before Xen Summit. Maybe Stefano can?

>> Also, do we need to update the advisory?
> 
> Dunno. I didn't do full analysis of what may go wrong, I've just worked
> my way far enough to understand what needs fixing. Whether an
> update is needed imo largely depends on whether the purpose of the
> patches wasn't fulfilled. People actually using XSM will notice very
> quickly that things don't work anymore, as can be seen from the
> osstest cases.

AFAICT, Arm does not seem to be affected by the problem (at least osstest does 
not complain). I would not expect x86 users to merge those patch, so maybe it 
should be ok.

Cheers,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH for-4.11 1/2] XSM: adjust Kconfig names

[Jan Beulich]

>>> On 18.06.19 at 16:44, <julien.grall@arm.com> wrote:
> On 18/06/2019 15:26, Jan Beulich wrote:
>> What I'd like to ask for in the future in any case though is that after
>> pushing stuff to stable trees you would please check the osstest
>> reports, and in case of regressions invest at least some time into
>> figuring out what broke. Right now, even with the XSM tests
>> (hopefully) taken care of there's still a flood of armhf failures, which
>> may or may not be due to environmental issues.
> 
> I usually look over osstest but fail to detect this was an issue because of the 
> XSAs. Regarding the other armhf failure, Ian already pointed out on IRC.
> 
> However, I will not have time to look at it before Xen Summit. Maybe Stefano 
> can?

Not before the summit? That's still almost a month out. We really want
to get 4.11.2 and also 4.10.4 out the door before that.

>>> Also, do we need to update the advisory?
>> 
>> Dunno. I didn't do full analysis of what may go wrong, I've just worked
>> my way far enough to understand what needs fixing. Whether an
>> update is needed imo largely depends on whether the purpose of the
>> patches wasn't fulfilled. People actually using XSM will notice very
>> quickly that things don't work anymore, as can be seen from the
>> osstest cases.
> 
> AFAICT, Arm does not seem to be affected by the problem (at least osstest does 
> not complain). I would not expect x86 users to merge those patch, so maybe it 
> should be ok.

Well, the breakage was in the one case where SILO mode actually
disallows what the test is specifically about - qemu running in a
stubdom, and hence needing to communicate with the actual guest.
I don't think there's any similar test to this for Arm in osstest.

Jan



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