[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v4 2/3] xsm: consolidate loading the policy buffer
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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |