On 31.05.2022 20:20, Daniel P. Smith wrote:
> Previously, initializing the policy buffer was split between two functions,
> xsm_{multiboot,dt}_policy_init() and xsm_core_init(). The latter for loading
> the policy from boot modules and the former for falling back to built-in
> policy.
>
> This patch moves all policy buffer initialization logic under the
> xsm_{multiboot,dt}_policy_init() functions. It then ensures that an error
> message is printed for every error condition that may occur in the functions.
> With all policy buffer init contained and only called when the policy buffer
> must be populated, the respective xsm_{mb,dt}_init() functions will panic for
> all errors except ENOENT. An ENOENT signifies that a policy file could not be
> located. Since it is not possible to know if late loading of the policy file
> is
> intended, a warning is reported and XSM initialization is continued.
Is it really not possible to know? flask_init() panics in the one case
where a policy is strictly required. And I'm not convinced it is
appropriate to issue both an error and a warning in most (all?) of the
other cases (and it should be at most one of the two anyway imo).
> --- a/xen/include/xsm/xsm.h
> +++ b/xen/include/xsm/xsm.h
> @@ -775,7 +775,7 @@ int xsm_multiboot_init(
> unsigned long *module_map, const multiboot_info_t *mbi);
> int xsm_multiboot_policy_init(
> unsigned long *module_map, const multiboot_info_t *mbi,
> - void **policy_buffer, size_t *policy_size);
> + const unsigned char *policy_buffer[], size_t *policy_size);
See my v3 comment on the use of [] here. And that comment was actually
made before you sent v4 (unlike the later comment on patch 1). Oh,
actually you did change this in the function definition, just not here.
> @@ -32,14 +32,21 @@
> #ifdef CONFIG_MULTIBOOT
> int __init xsm_multiboot_policy_init(
> unsigned long *module_map, const multiboot_info_t *mbi,
> - void **policy_buffer, size_t *policy_size)
> + const unsigned char **policy_buffer, size_t *policy_size)
> {
> int i;
> module_t *mod = (module_t *)__va(mbi->mods_addr);
> - int rc = 0;
> + int rc = -ENOENT;
> u32 *_policy_start;
> unsigned long _policy_len;
>
> +#ifdef CONFIG_XSM_FLASK_POLICY
> + /* Initially set to builtin policy, overriden if boot module is found. */
Nit: "overridden"
Jan
