Re: [RFC PATCH] xen/flask: estimate max sidtable size

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.06.2025 10:55, Sergiy Kibrik wrote:
> 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.
> Thus in Xen the number of allocated entries in sidtable is hard-limited by 
> UINT_MAX.
> 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 amounts. So we seek to estimate this limit.
> 
> This patch suggests one way to do it using Xen's flask policy.
> List of users, roles and types is read from binary policy using setools utils,
> then it is used to count the No. of combinations these values can give.
> This No. of combinations then can be used in code as a practical replacement
> of UINT_MAX limit. Also it can be used later to pre-allocate sidtable at boot
> and avoid runtime entries allocation altogether.
> 
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@xxxxxxxx>
> ---
> This RFC presents a concept of estimating a max possible sidtable size.
> Can we discuss how valid this concept is? Currently it yields 420 as max SID,
> is it a reasonable number?

As this is policy dependent - what policy did you use to obtain that 420?

> --- a/xen/xsm/flask/Makefile
> +++ b/xen/xsm/flask/Makefile
> @@ -14,7 +14,7 @@ AV_H_DEPEND := $(srcdir)/policy/access_vectors
>  
>  FLASK_H_FILES := flask.h class_to_string.h initial_sid_to_string.h
>  AV_H_FILES := av_perm_to_string.h av_permissions.h
> -ALL_H_FILES := $(addprefix include/,$(FLASK_H_FILES) $(AV_H_FILES))
> +ALL_H_FILES := $(addprefix include/,$(FLASK_H_FILES) $(AV_H_FILES) 
> se_limits.h)

As a nit: Dashes in preference to underscores please in any new files'
names.

> @@ -54,4 +54,7 @@ $(obj)/policy.bin: FORCE
>               FLASK_BUILD_DIR=$(FLASK_BUILD_DIR) POLICY_FILENAME=$(POLICY_SRC)
>       cmp -s $(POLICY_SRC) $@ || cp $(POLICY_SRC) $@
>  
> +$(obj)/%/se_limits.h: $(obj)/policy.bin
> +     $(srcdir)/policy/mkselim.sh $^ $@

Hmm, that's using the built-in policy, isn't it? What if later another
policy is loaded? Wouldn't it be possible to have ...

> --- a/xen/xsm/flask/ss/sidtab.c
> +++ b/xen/xsm/flask/ss/sidtab.c
> @@ -13,6 +13,7 @@
>  #include "flask.h"
>  #include "security.h"
>  #include "sidtab.h"
> +#include "se_limits.h"
>  
>  #define SIDTAB_HASH(sid) ((sid) & SIDTAB_HASH_MASK)
>  
> @@ -228,7 +229,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 == SEPOL_SID_LIMIT || s->shutdown )

... more than this many SIDs? What if CONFIG_XSM_FLASK_POLICY isn't even set?

It also doesn't really become clear to me how you avoid or even (meaningfully)
bound memory allocation here. A table of several hundred entries is still a
decent size. If you really knew the max size up front, why couldn't the table
be allocated statically. (Sadly the table allocation isn't in context, as you
don't even touch that code, wherever it lives.)

Jan