[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v4 2/3] xsm: consolidate loading the policy buffer


  • To: "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 2 Jun 2022 11:47:14 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=R2nAychlMBFxvawYdIVjlApNpLN1F+sVNEB0C89rDOo=; b=GaEKOKI7zpsfKX0ThU+889UHuGRGmQnVBYvV6n97j2NAYnuQy+byl9G00NonCHg0V7oxGbDBqFONRoA9Qi4FwxjFyJ3WR0eJ2A4Lh+1hxUqba19mraddlzL+BSmI1gswU11I6v7sDHE4dMYShDygajxMNrlYSg7EJF8cshHAvE62mMZ7ZYvlokZjl4BptYoC4ereobI6b/bAm2oMmQSk26F8J0D8BUxRocRx4EFOGrzivFZq8EaNmBE+5wjSpW2a+miLi8R86OYLoAcdfGtfMcEd31NAm8k+lw5Dn1G7DroWRhq6ekbg9VUrPgpOqiaITuqW42G9EnwTXYaYmx36VA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=jo8od4Dg3qGxAXEzUfChc7PvPm3Uk5AC7Tsz00WcN2qIi0Om40nHJssZaW74DGNQHa8U5oaKZDRC6UyK9X9HJliNqNGPDmsk+YOEDj9lTuqogkDxQlDP7QNkbEuQf0A2v9z5xLDwzdE4y5dlr10hrolpgY2amOkUA2aE70w0QafeKqKzVfpYXxbdMsyF8AVPhp1fDAfRcENAzRmR1DGd97AEzW3bKrpPVeCDoYkOoOAvN7l2Sa+64zELFUZg7A9Gm7sR0qwv5r9YS5zCXCPYMXdngD4L815+or9jYwjl/FgzDeopX0ZcQxQhIIrJM5Wkn885wfExZCW2RSgbpLjhpA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: scott.davis@xxxxxxxxxx, christopher.clark@xxxxxxxxxx, jandryuk@xxxxxxxxx, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Thu, 02 Jun 2022 09:47:30 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.