On 03/04/2016 04:07 PM, Andrew Morton wrote:
On Fri, 4 Mar 2016 15:50:48 -0800 Laura Abbott <[email protected]> wrote:By default, page poisoning uses a poison value (0xaa) on free. If this is changed to 0, the page is not only sanitized but zeroing on alloc with __GFP_ZERO can be skipped as well. The tradeoff is that detecting corruption from the poisoning is harder to detect. This feature also cannot be used with hibernation since pages are not guaranteed to be zeroed after hibernation. Credit to Grsecurity/PaX team for inspiring this work --- a/kernel/power/hibernate.c +++ b/kernel/power/hibernate.c @@ -1158,6 +1158,22 @@ static int __init kaslr_nohibernate_setup(char *str) return nohibernate_setup(str); } +static int __init page_poison_nohibernate_setup(char *str) +{ +#ifdef CONFIG_PAGE_POISONING_ZERO + /* + * The zeroing option for page poison skips the checks on alloc. + * since hibernation doesn't save free pages there's no way to + * guarantee the pages will still be zeroed. + */ + if (!strcmp(str, "on")) { + pr_info("Disabling hibernation due to page poisoning\n"); + return nohibernate_setup(str); + } +#endif + return 1; +}It seems a bit unfriendly to silently accept the boot option but not actually do anything with it. Perhaps a `#else pr_info("sorry")' is needed. But I bet we made the same mistake in 1000 other places. What happens if page_poison_nohibernate_setup() simply doesn't exist when CONFIG_PAGE_POISONING_ZERO=n? It looks like kernel/params.c:parse_args() says "Unknown parameter".
I didn't see that behavior when I tested, even with nonsense parameters. It looks like it might fall back to some other behavior before giving -ENOENT? It's also worth noting the page_poison= option is also parsed in mm/page_poison.c to do other on/off of the poisoning feature. The option code supported it and it seemed to match better with what the existing hibernate code was doing with turning off options. Thanks, Laura
