[v2] xen/flask: limit sidtable size

[XEN PATCH v2] xen/flask: limit sidtable size

Posted by Sergiy Kibrik 1 month, 4 weeks ago

Currently Xen lacks a defined largest number of security IDs it can potentially
use. The number of SIDs are naturally limited by number of security contexts
provided by a given security policy, i.e. how many combination of user, role
and type there can be, and is dependant on the policy being used.
Since the policy is generally not known in advance the size of sidtable in Xen
has a rather high limit of UINT_MAX entries.

However in the embedded environment configured for safety it is desirable to
avoid guest-triggered dynamic memory allocations at runtime, or at least limit
them to some decent and predictable amounts. This patch provides a configuration
option to impose such a limit.

Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
CC: Jan Beulich <jbeulich@suse.com>
---
changes in v2:
  - use one config option instead of 2
  - use base 2 exponent

patch v1 here:
   https://lore.kernel.org/xen-devel/20250822095123.998313-1-Sergiy_Kibrik@epam.com/

 -Sergiy
---
 xen/common/Kconfig        | 11 +++++++++++
 xen/xsm/flask/ss/sidtab.c |  4 +++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 76f9ce705f..83bc9870dc 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -418,6 +418,17 @@ config XSM_FLASK_AVC_STATS
 
 	  If unsure, say Y.
 
+config XSM_FLASK_SIDTABLE_ORDER
+	int "Maximum number of security identifiers (base-2 exponent)" if EXPERT
+	range 4 32
+	default 32
+	depends on XSM_FLASK
+	help
+	  Limit the number of security identifiers allocated and operated by Xen.
+	  The value is a base-2 exponent. This will set the max number of SIDs
+	  and hence the max number of security contexts and heap memory
+	  allocated for SID table entries.
+
 config XSM_FLASK_POLICY
 	bool "Compile Xen with a built-in FLASK security policy"
 	default y if "$(XEN_HAS_CHECKPOLICY)" = "y"
diff --git a/xen/xsm/flask/ss/sidtab.c b/xen/xsm/flask/ss/sidtab.c
index 69fc3389b3..0081abdc86 100644
--- a/xen/xsm/flask/ss/sidtab.c
+++ b/xen/xsm/flask/ss/sidtab.c
@@ -14,6 +14,8 @@
 #include "security.h"
 #include "sidtab.h"
 
+#define SID_LIMIT ((1UL << CONFIG_XSM_FLASK_SIDTABLE_ORDER) - 1)
+
 #define SIDTAB_HASH(sid) ((sid) & SIDTAB_HASH_MASK)
 
 #define INIT_SIDTAB_LOCK(s) spin_lock_init(&(s)->lock)
@@ -228,7 +230,7 @@ int sidtab_context_to_sid(struct sidtab *s, struct context *context,
         if ( sid )
             goto unlock_out;
         /* No SID exists for the context.  Allocate a new one. */
-        if ( s->next_sid == UINT_MAX || s->shutdown )
+        if ( s->next_sid == SID_LIMIT || s->shutdown )
         {
             ret = -ENOMEM;
             goto unlock_out;
-- 
2.25.1

Re: [XEN PATCH v2] xen/flask: limit sidtable size

Posted by Jan Beulich 1 month, 4 weeks ago

On 01.09.2025 12:52, Sergiy Kibrik wrote:
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -418,6 +418,17 @@ config XSM_FLASK_AVC_STATS
>  
>  	  If unsure, say Y.
>  
> +config XSM_FLASK_SIDTABLE_ORDER
> +	int "Maximum number of security identifiers (base-2 exponent)" if EXPERT
> +	range 4 32
> +	default 32

When 32 is chosen (i.e. also the default when the prompt is hidden), ...

> --- a/xen/xsm/flask/ss/sidtab.c
> +++ b/xen/xsm/flask/ss/sidtab.c
> @@ -14,6 +14,8 @@
>  #include "security.h"
>  #include "sidtab.h"
>  
> +#define SID_LIMIT ((1UL << CONFIG_XSM_FLASK_SIDTABLE_ORDER) - 1)

... for Arm32 I expect either already the compiler will not like this construct,
or the latest an UBSAN checker would object.

Jan

Re: [XEN PATCH v2] xen/flask: limit sidtable size

Posted by Daniel P. Smith 1 month, 3 weeks ago

Hi Sergiy,

If you don't mind, please CC me directly, as I am the only XSM 
maintainer for which you will need my Ack. And for whatever reason, I 
cannot find the v2 post in my xen-devel folder. If you want to resend me 
v2, it would be greatly appreciated.

V/r,
Daniel P. Smith
Apertus Solutions, LLC

On 9/2/25 05:41, Jan Beulich wrote:
> On 01.09.2025 12:52, Sergiy Kibrik wrote:
>> --- a/xen/common/Kconfig
>> +++ b/xen/common/Kconfig
>> @@ -418,6 +418,17 @@ config XSM_FLASK_AVC_STATS
>>   
>>   	  If unsure, say Y.
>>   
>> +config XSM_FLASK_SIDTABLE_ORDER
>> +	int "Maximum number of security identifiers (base-2 exponent)" if EXPERT
>> +	range 4 32
>> +	default 32
> When 32 is chosen (i.e. also the default when the prompt is hidden), ...
>
>> --- a/xen/xsm/flask/ss/sidtab.c
>> +++ b/xen/xsm/flask/ss/sidtab.c
>> @@ -14,6 +14,8 @@
>>   #include "security.h"
>>   #include "sidtab.h"
>>   
>> +#define SID_LIMIT ((1UL << CONFIG_XSM_FLASK_SIDTABLE_ORDER) - 1)
> ... for Arm32 I expect either already the compiler will not like this construct,
> or the latest an UBSAN checker would object.
>
> Jan

Re: [XEN PATCH v2] xen/flask: limit sidtable size

Posted by Sergiy Kibrik 1 month ago

06.09.25 01:01, Daniel P. Smith:
> Hi Sergiy,
> 
> If you don't mind, please CC me directly, as I am the only XSM 
> maintainer for which you will need my Ack. And for whatever reason, I 
> cannot find the v2 post in my xen-devel folder. If you want to resend me 
> v2, it would be greatly appreciated.
> 

yes, sure

> 
> On 9/2/25 05:41, Jan Beulich wrote:
>> On 01.09.2025 12:52, Sergiy Kibrik wrote:
>>> --- a/xen/common/Kconfig
>>> +++ b/xen/common/Kconfig
>>> @@ -418,6 +418,17 @@ config XSM_FLASK_AVC_STATS
>>>   
>>>   	  If unsure, say Y.
>>>   
>>> +config XSM_FLASK_SIDTABLE_ORDER
>>> +	int "Maximum number of security identifiers (base-2 exponent)" if EXPERT
>>> +	range 4 32
>>> +	default 32
>> When 32 is chosen (i.e. also the default when the prompt is hidden), ...
>>
>>> --- a/xen/xsm/flask/ss/sidtab.c
>>> +++ b/xen/xsm/flask/ss/sidtab.c
>>> @@ -14,6 +14,8 @@
>>>   #include "security.h"
>>>   #include "sidtab.h"
>>>   
>>> +#define SID_LIMIT ((1UL << CONFIG_XSM_FLASK_SIDTABLE_ORDER) - 1)
>> ... for Arm32 I expect either already the compiler will not like this construct,
>> or the latest an UBSAN checker would object.
>>

you're right, arm32 toolchain is not building this.
Would the following be acceptable then? :

#define SID_LIMIT ((1ULL << CONFIG_XSM_FLASK_SIDTABLE_ORDER) - 1)

   -Sergiy

Re: [XEN PATCH v2] xen/flask: limit sidtable size

Posted by Jan Beulich 1 month ago

On 26.09.2025 08:32, Sergiy Kibrik wrote:
> 06.09.25 01:01, Daniel P. Smith:
>> On 9/2/25 05:41, Jan Beulich wrote:
>>> On 01.09.2025 12:52, Sergiy Kibrik wrote:
>>>> --- a/xen/common/Kconfig
>>>> +++ b/xen/common/Kconfig
>>>> @@ -418,6 +418,17 @@ config XSM_FLASK_AVC_STATS
>>>>   
>>>>   	  If unsure, say Y.
>>>>   
>>>> +config XSM_FLASK_SIDTABLE_ORDER
>>>> +	int "Maximum number of security identifiers (base-2 exponent)" if EXPERT
>>>> +	range 4 32
>>>> +	default 32
>>> When 32 is chosen (i.e. also the default when the prompt is hidden), ...
>>>
>>>> --- a/xen/xsm/flask/ss/sidtab.c
>>>> +++ b/xen/xsm/flask/ss/sidtab.c
>>>> @@ -14,6 +14,8 @@
>>>>   #include "security.h"
>>>>   #include "sidtab.h"
>>>>   
>>>> +#define SID_LIMIT ((1UL << CONFIG_XSM_FLASK_SIDTABLE_ORDER) - 1)
>>> ... for Arm32 I expect either already the compiler will not like this construct,
>>> or the latest an UBSAN checker would object.
> 
> you're right, arm32 toolchain is not building this.
> Would the following be acceptable then? :
> 
> #define SID_LIMIT ((1ULL << CONFIG_XSM_FLASK_SIDTABLE_ORDER) - 1)

Personally I'd consider this an abuse of the ULL suffix. But it'll be Daniel
to judge in the end.

Jan