[PATCH 1/2] xsm: move the XSM_MAGIC value to Kconfig

[PATCH 1/2] xsm: move the XSM_MAGIC value to Kconfig

[Doug Goldstein]

Let Kconfig set the XSM_MAGIC value for us.

Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
---
CC: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 xen/common/Kconfig       | 8 ++++++++
 xen/include/xen/config.h | 1 -
 xen/include/xsm/xsm.h    | 5 +----
 xen/xsm/xsm_core.c       | 4 ++--
 xen/xsm/xsm_policy.c     | 6 +++---
 5 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 8fbc46d..d661da3 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -113,6 +113,14 @@ config XSM
 
 	  If unsure, say N.
 
+# XSM magic for policy detection
+config XSM_MAGIC
+    hex
+    default 0xf97cff8c if FLASK
+    default 0 if !FLASK
+    ---help---
+      Identifies a FLASK XSM policy start point
+
 # Enable schedulers
 menu "Schedulers"
 	visible if EXPERT = "y"
diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h
index 96f5539..3f8c53d 100644
--- a/xen/include/xen/config.h
+++ b/xen/include/xen/config.h
@@ -79,7 +79,6 @@
 #define STR(...) __STR(__VA_ARGS__)
 
 #ifdef CONFIG_FLASK
-#define XSM_MAGIC 0xf97cff8c
 /* Maintain statistics on the access vector cache */
 #define FLASK_AVC_STATS 1
 #endif
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 3afed70..7f313ad 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -21,11 +21,8 @@
 typedef void xsm_op_t;
 DEFINE_XEN_GUEST_HANDLE(xsm_op_t);
 
-/* policy magic number (defined by XSM_MAGIC) */
+/* policy magic number (defined by CONFIG_XSM_MAGIC) */
 typedef u32 xsm_magic_t;
-#ifndef XSM_MAGIC
-#define XSM_MAGIC 0x00000000
-#endif
 
 /* These annotations are used by callers and in dummy.h to document the
  * default actions of XSM hooks. They should be compiled out otherwise.
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 5e432de..d6965ba 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -67,7 +67,7 @@ int __init xsm_multiboot_init(unsigned long *module_map,
 
     printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n");
 
-    if ( XSM_MAGIC )
+    if ( CONFIG_XSM_MAGIC )
     {
         ret = xsm_multiboot_policy_init(module_map, mbi, bootstrap_map);
         if ( ret )
@@ -92,7 +92,7 @@ int __init xsm_dt_init(void)
 
     printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n");
 
-    if ( XSM_MAGIC )
+    if ( CONFIG_XSM_MAGIC )
     {
         ret = xsm_dt_policy_init();
         if ( ret )
diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c
index b60d822..52aa4a9 100644
--- a/xen/xsm/xsm_policy.c
+++ b/xen/xsm/xsm_policy.c
@@ -54,7 +54,7 @@ int __init xsm_multiboot_policy_init(unsigned long *module_map,
         _policy_start = bootstrap_map(mod + i);
         _policy_len   = mod[i].mod_end;
 
-        if ( (xsm_magic_t)(*_policy_start) == XSM_MAGIC )
+        if ( (xsm_magic_t)(*_policy_start) == CONFIG_XSM_MAGIC )
         {
             policy_buffer = (char *)_policy_start;
             policy_size = _policy_len;
@@ -89,10 +89,10 @@ int __init xsm_dt_policy_init(void)
 
     copy_from_paddr(&magic, paddr, sizeof(magic));
 
-    if ( magic != XSM_MAGIC )
+    if ( magic != CONFIG_XSM_MAGIC )
     {
         printk(XENLOG_ERR "xsm: Invalid magic for XSM blob got 0x%x "
-               "expected 0x%x\n", magic, XSM_MAGIC);
+               "expected 0x%x\n", magic, CONFIG_XSM_MAGIC);
         return -EINVAL;
     }
 
-- 
2.4.10


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

[PATCH 2/2] xsm: move FLASK_AVC_STATS to Kconfig

[Doug Goldstein]

Have Kconfig set CONFIG_FLASK_AVC_STATS and prefix all uses with CONFIG_
to use the Kconfig variable.

Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
---
CC: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 xen/common/Kconfig          | 8 +++++++-
 xen/include/xen/config.h    | 5 -----
 xen/xsm/flask/avc.c         | 4 ++--
 xen/xsm/flask/flask_op.c    | 4 ++--
 xen/xsm/flask/include/avc.h | 2 +-
 5 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index d661da3..db23edc 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -23,6 +23,12 @@ config FLASK
 
 	  If unsure, say N.
 
+config FLASK_AVC_STATS
+    def_bool y if FLASK
+    depends on FLASK
+    ---help---
+      Maintain statistics on the access vector cache
+
 # Select HAS_DEVICE_TREE if device tree is supported
 config HAS_DEVICE_TREE
 	bool
@@ -117,7 +123,7 @@ config XSM
 config XSM_MAGIC
     hex
     default 0xf97cff8c if FLASK
-    default 0 if !FLASK
+    default 0
     ---help---
       Identifies a FLASK XSM policy start point
 
diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h
index 3f8c53d..ef6e5ee 100644
--- a/xen/include/xen/config.h
+++ b/xen/include/xen/config.h
@@ -78,11 +78,6 @@
 #define __STR(...) #__VA_ARGS__
 #define STR(...) __STR(__VA_ARGS__)
 
-#ifdef CONFIG_FLASK
-/* Maintain statistics on the access vector cache */
-#define FLASK_AVC_STATS 1
-#endif
-
 /* allow existing code to work with Kconfig variable */
 #define NR_CPUS CONFIG_NR_CPUS
 
diff --git a/xen/xsm/flask/avc.c b/xen/xsm/flask/avc.c
index 31bc702..7764379 100644
--- a/xen/xsm/flask/avc.c
+++ b/xen/xsm/flask/avc.c
@@ -56,7 +56,7 @@ const struct selinux_class_perm selinux_class_perm = {
 #define AVC_DEF_CACHE_THRESHOLD        512
 #define AVC_CACHE_RECLAIM        16
 
-#ifdef FLASK_AVC_STATS
+#ifdef CONFIG_FLASK_AVC_STATS
 #define avc_cache_stats_incr(field)                 \
 do {                                \
     __get_cpu_var(avc_cache_stats).field++;        \
@@ -101,7 +101,7 @@ struct avc_callback_node {
 /* Exported via Flask hypercall */
 unsigned int avc_cache_threshold = AVC_DEF_CACHE_THRESHOLD;
 
-#ifdef FLASK_AVC_STATS
+#ifdef CONFIG_FLASK_AVC_STATS
 DEFINE_PER_CPU(struct avc_cache_stats, avc_cache_stats);
 #endif
 
diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
index f4f5dd1..3c9c99e 100644
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -469,7 +469,7 @@ static int flask_security_make_bools(void)
     return ret;
 }
 
-#ifdef FLASK_AVC_STATS
+#ifdef CONFIG_FLASK_AVC_STATS
 
 static int flask_security_avc_cachestats(struct xen_flask_cache_stats *arg)
 {
@@ -761,7 +761,7 @@ ret_t do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op)
         rv = avc_get_hash_stats(&op.u.hash_stats);
         break;
 
-#ifdef FLASK_AVC_STATS
+#ifdef CONFIG_FLASK_AVC_STATS
     case FLASK_AVC_CACHESTATS:
         rv = flask_security_avc_cachestats(&op.u.cache_stats);
         break;
diff --git a/xen/xsm/flask/include/avc.h b/xen/xsm/flask/include/avc.h
index 4283562..729856e 100644
--- a/xen/xsm/flask/include/avc.h
+++ b/xen/xsm/flask/include/avc.h
@@ -108,7 +108,7 @@ struct xen_flask_hash_stats;
 int avc_get_hash_stats(struct xen_flask_hash_stats *arg);
 extern unsigned int avc_cache_threshold;
 
-#ifdef FLASK_AVC_STATS
+#ifdef CONFIG_FLASK_AVC_STATS
 DECLARE_PER_CPU(struct avc_cache_stats, avc_cache_stats);
 #endif
 
-- 
2.4.10


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

Re: [PATCH 2/2] xsm: move FLASK_AVC_STATS to Kconfig

[Jan Beulich]

>>> On 07.03.16 at 19:42, <cardoe@cardoe.com> wrote:
> Have Kconfig set CONFIG_FLASK_AVC_STATS and prefix all uses with CONFIG_
> to use the Kconfig variable.

Same question here: What's the benefit of doing it this way?

> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -23,6 +23,12 @@ config FLASK
>  
>  	  If unsure, say N.
>  
> +config FLASK_AVC_STATS
> +    def_bool y if FLASK
> +    depends on FLASK

With this "depends" the "if FLASK" is pointless.

Also (again) - indentation.

Jan


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

Re: [PATCH 2/2] xsm: move FLASK_AVC_STATS to Kconfig

[Daniel De Graaf]

On 03/08/2016 04:46 AM, Jan Beulich wrote:
>>>> On 07.03.16 at 19:42, <cardoe@cardoe.com> wrote:
>> Have Kconfig set CONFIG_FLASK_AVC_STATS and prefix all uses with CONFIG_
>> to use the Kconfig variable.
>
> Same question here: What's the benefit of doing it this way?

This removes the stats tracking, which might (I have not tested) speed up
the security server by avoiding the __get_cpu_var call and increment. The
corresponding SELinux knob is a Kconfig option in Linux.

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

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

Re: [PATCH 2/2] xsm: move FLASK_AVC_STATS to Kconfig

[Jan Beulich]

>>> On 08.03.16 at 17:22, <dgdegra@tycho.nsa.gov> wrote:
> On 03/08/2016 04:46 AM, Jan Beulich wrote:
>>>>> On 07.03.16 at 19:42, <cardoe@cardoe.com> wrote:
>>> Have Kconfig set CONFIG_FLASK_AVC_STATS and prefix all uses with CONFIG_
>>> to use the Kconfig variable.
>>
>> Same question here: What's the benefit of doing it this way?
> 
> This removes the stats tracking, which might (I have not tested) speed up
> the security server by avoiding the __get_cpu_var call and increment.

No, I don not think the patch removes anything. The Kconfig option
doesn't have a prompt. But anyway, ...

> The
> corresponding SELinux knob is a Kconfig option in Linux.
> 
> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

... if you're fine with it, we'll put it in (once the mechanical issues
got addressed).

Jan


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

Re: [PATCH 2/2] xsm: move FLASK_AVC_STATS to Kconfig

[Daniel De Graaf]

On 03/08/2016 11:51 AM, Jan Beulich wrote:
>>>> On 08.03.16 at 17:22, <dgdegra@tycho.nsa.gov> wrote:
>> On 03/08/2016 04:46 AM, Jan Beulich wrote:
>>>>>> On 07.03.16 at 19:42, <cardoe@cardoe.com> wrote:
>>>> Have Kconfig set CONFIG_FLASK_AVC_STATS and prefix all uses with CONFIG_
>>>> to use the Kconfig variable.
>>>
>>> Same question here: What's the benefit of doing it this way?
>>
>> This removes the stats tracking, which might (I have not tested) speed up
>> the security server by avoiding the __get_cpu_var call and increment.
>
> No, I don not think the patch removes anything. The Kconfig option
> doesn't have a prompt. But anyway, ...

Ah, I missed that: I saw the --help-- line and assumed it was the prompt.
Either way, this #define is a configuration-like knob that doesn't need to
be hard-coded in a header as it currently is.

>
>> The
>> corresponding SELinux knob is a Kconfig option in Linux.
>>
>> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>
> ... if you're fine with it, we'll put it in (once the mechanical issues
> got addressed).

-- 
Daniel De Graaf
National Security Agency

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

Re: [PATCH 2/2] xsm: move FLASK_AVC_STATS to Kconfig

[Doug Goldstein]

[-- Attachment #1.1.1: Type: text/plain, Size: 1364 bytes --]

On 3/8/16 12:01 PM, Daniel De Graaf wrote:
> On 03/08/2016 11:51 AM, Jan Beulich wrote:
>>>>> On 08.03.16 at 17:22, <dgdegra@tycho.nsa.gov> wrote:
>>> On 03/08/2016 04:46 AM, Jan Beulich wrote:
>>>>>>> On 07.03.16 at 19:42, <cardoe@cardoe.com> wrote:
>>>>> Have Kconfig set CONFIG_FLASK_AVC_STATS and prefix all uses with
>>>>> CONFIG_
>>>>> to use the Kconfig variable.
>>>>
>>>> Same question here: What's the benefit of doing it this way?
>>>
>>> This removes the stats tracking, which might (I have not tested)
>>> speed up
>>> the security server by avoiding the __get_cpu_var call and increment.
>>
>> No, I don not think the patch removes anything. The Kconfig option
>> doesn't have a prompt. But anyway, ...
> 
> Ah, I missed that: I saw the --help-- line and assumed it was the prompt.
> Either way, this #define is a configuration-like knob that doesn't need to
> be hard-coded in a header as it currently is.
> 
>>
>>> The
>>> corresponding SELinux knob is a Kconfig option in Linux.
>>>
>>> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>>
>> ... if you're fine with it, we'll put it in (once the mechanical issues
>> got addressed).
> 

Daniel,

Would you like me to make this a real configuration option? Or proceed
with the current path and we can make a configuration option later?

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH 2/2] xsm: move FLASK_AVC_STATS to Kconfig

[Doug Goldstein]

[-- Attachment #1.1.1: Type: text/plain, Size: 1542 bytes --]

On 3/14/16 9:05 AM, Doug Goldstein wrote:
> On 3/8/16 12:01 PM, Daniel De Graaf wrote:
>> On 03/08/2016 11:51 AM, Jan Beulich wrote:
>>>>>> On 08.03.16 at 17:22, <dgdegra@tycho.nsa.gov> wrote:
>>>> On 03/08/2016 04:46 AM, Jan Beulich wrote:
>>>>>>>> On 07.03.16 at 19:42, <cardoe@cardoe.com> wrote:
>>>>>> Have Kconfig set CONFIG_FLASK_AVC_STATS and prefix all uses with
>>>>>> CONFIG_
>>>>>> to use the Kconfig variable.
>>>>>
>>>>> Same question here: What's the benefit of doing it this way?
>>>>
>>>> This removes the stats tracking, which might (I have not tested)
>>>> speed up
>>>> the security server by avoiding the __get_cpu_var call and increment.
>>>
>>> No, I don not think the patch removes anything. The Kconfig option
>>> doesn't have a prompt. But anyway, ...
>>
>> Ah, I missed that: I saw the --help-- line and assumed it was the prompt.
>> Either way, this #define is a configuration-like knob that doesn't need to
>> be hard-coded in a header as it currently is.
>>
>>>
>>>> The
>>>> corresponding SELinux knob is a Kconfig option in Linux.
>>>>
>>>> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>>>
>>> ... if you're fine with it, we'll put it in (once the mechanical issues
>>> got addressed).
>>
> 
> Daniel,
> 
> Would you like me to make this a real configuration option? Or proceed
> with the current path and we can make a configuration option later?
> 

I'll resubmit as is and we can address making that a user facing option
in a follow on.

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH 1/2] xsm: move the XSM_MAGIC value to Kconfig

[Jan Beulich]

>>> On 07.03.16 at 19:42, <cardoe@cardoe.com> wrote:
> Let Kconfig set the XSM_MAGIC value for us.

What's the benefit of doing this at the Kconfig layer?

> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -113,6 +113,14 @@ config XSM
>  
>  	  If unsure, say N.
>  
> +# XSM magic for policy detection
> +config XSM_MAGIC
> +    hex
> +    default 0xf97cff8c if FLASK
> +    default 0 if !FLASK

This second "if" is pointless.

Also note the broken indentation (using spaces instead of a tab).

Jan


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

Re: [PATCH 1/2] xsm: move the XSM_MAGIC value to Kconfig

[Daniel De Graaf]

On 03/07/2016 01:42 PM, Doug Goldstein wrote:
> Let Kconfig set the XSM_MAGIC value for us.
>
> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>

This is not the best place to define this constant: it doesn't
make sense for it to be user-configurable.  If you want to move it
out of config.h, I think the best solution is to define XSM_MAGIC
inside xsm/xsm.h depending on the value of CONFIG_FLASK.


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