Re: [Xen-devel] [PATCH] mm: option to _always_ scrub freed domheap pages

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 5/6/19 1:46 PM, Eslam Elnikety wrote:
> Give the administrator further control on when to scrub domheap pages by 
> adding
> an option to always scrub. This is a safety feature that, when enabled,
> prevents a (buggy) domain from leaking secrets if it accidentally frees a page
> without proper scrubbing.
> 
> Signed-off-by: Eslam Elnikety <elnikety@xxxxxxxxxx>

Now that I think about it -- Andy, isn't there a patch in the XenServer
patchqueue to enable scrubbing by default?

I'm wondering if this should default to 'true', and people who really
want the extra performance should turn it off.

Only one other minor comment:

> ---
>  docs/misc/xen-command-line.pandoc |  8 ++++++++
>  xen/common/page_alloc.c           | 11 +++++++++--
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/misc/xen-command-line.pandoc 
> b/docs/misc/xen-command-line.pandoc
> index 7dcb22932a..5a92949c5a 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -270,6 +270,14 @@ and not running softirqs. Reduce this if softirqs are 
> not being run frequently
>  enough. Setting this to a high value may cause boot failure, particularly if
>  the NMI watchdog is also enabled.
>  
> +### scrub_domheap
> +> `= <boolean>`
> +
> +> Default: `false`
> +
> +Scrub domains' freed pages. This is a safety net against a (buggy) domain
> +accidentally leaking secrets by releasing pages without proper sanitization.
> +
>  ### clocksource (x86)
>  > `= pit | hpet | acpi | tsc`
>  
> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> index be44158033..678a00ac9b 100644
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -214,6 +214,12 @@ custom_param("bootscrub", parse_bootscrub_param);
>  static unsigned long __initdata opt_bootscrub_chunk = MB(128);
>  size_param("bootscrub_chunk", opt_bootscrub_chunk);
>  
> +/*
> + * scrub_domheap -> Domheap pages are scrubbed when freed
> + */
> +static bool_t opt_scrub_domheap = 0;
> +boolean_param("scrub_domheap", opt_scrub_domheap);

I'm sure Jan will request this to be 'scrub-domheap' instead (not using
'_' when you can use '-').

Otherwise this looks good to me:

Acked-by: George Dunlap <george.dunlap@xxxxxxxxxx>

I think both of these could probably be fixed up on check-in.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel