Re: [XEN PATCH v2] xen: allow XSM_FLASK_POLICY only if checkpolicy binary is available

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.07.2021 14:38, Anthony PERARD wrote:
> This will help prevent the CI loop from having build failures when
> `checkpolicy` isn't available, when doing "randconfig" jobs.
> 
> Also, move the check out of Config.mk and into xen/ build system.
> Nothing in tools/ is using that information as it's done by
> ./configure.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> ---
> We might want to have a new Makefile for this kind of check that
> Kconfig is going to use, just to keep the main Makefile a bit cleaner.
> But maybe another time, if more are comming.
> 
> v2:
> - move check to Makefile

I'm afraid I don't understand:

> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -17,6 +17,8 @@ export XEN_BUILD_HOST       ?= $(shell hostname)
>  PYTHON_INTERPRETER   := $(word 1,$(shell which python3 python python2 
> 2>/dev/null) python)
>  export PYTHON                ?= $(PYTHON_INTERPRETER)
>  
> +export CHECKPOLICY   ?= checkpolicy
> +
>  export BASEDIR := $(CURDIR)
>  export XEN_ROOT := $(BASEDIR)/..
>  
> @@ -156,6 +158,8 @@ CFLAGS += $(CLANG_FLAGS)
>  export CLANG_FLAGS
>  endif
>  
> +export HAS_CHECKPOLICY := $(call success,$(CHECKPOLICY) -h 2>&1 | grep -q 
> xen)

While the setting indeed gets obtained in a Makefile now, ...

> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -235,8 +235,8 @@ config XSM_FLASK_AVC_STATS
>  
>  config XSM_FLASK_POLICY
>       bool "Compile Xen with a built-in FLASK security policy"
> -     default y if "$(XEN_HAS_CHECKPOLICY)" = "y"
> -     depends on XSM_FLASK
> +     default y
> +     depends on XSM_FLASK && "$(HAS_CHECKPOLICY)"

... it's still used as a Kconfig dependency. This in particular
does not address George's concern about a setting silently getting
turned off behind the back of the person having enabled it (and
this could happen at any time, not just during the initial build,
where one might still remember to diff .config against
.config.old). The minimal thing imo is some kind of warning then.
Even better would be if the setting was left unchanged and the
build would fail; a solution for randconfig would then still be
needed of course. If we wanted to go this route, a tristate type
for the values may be unavoidable, along the lines of what Jürgen
has suggested. I wouldn't think of a global override though, but
a distinction of the origin of each option's setting. This might
be as simple as y vs Y for "positive" values and "# CONFIG_...
is not set" present (for visible options) or absent (for options
the user can't control) for "negative" ones. But yes, this would
likely be an intrusive change _and_ it would not be clear how to
transform existing .config-s, so is unlikely to be suitable for
the immediate issue at hand.

Jan