| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <201107290001.14465.rjw@sisk.pl>
Date: Fri, 29 Jul 2011 00:01:14 +0200
From: "Rafael J. Wysocki" <rjw@...k.pl>
To: Martin Schwidefsky <schwidefsky@...ibm.com>
Cc: linux-pm@...ts.linux-foundation.org, linux-kernel@...r.kernel.org,
linux-s390@...r.kernel.org, Pavel Machek <pavel@....cz>,
Jiri Slaby <jslaby@...e.cz>
Subject: Re: [patch 1/1] [PATCH] include storage keys in hibernation image.
Hi,
Sorry for the extreme delay.
On Wednesday, June 08, 2011, Martin Schwidefsky wrote:
> From: Martin Schwidefsky <schwidefsky@...ibm.com>
>
> For s390 there is one additional byte associated with each page,
> the storage key. This byte contains the referenced and changed
> bits and needs to be included into the hibernation image.
> If the storage keys are not restored to their previous state all
> original pages would appear to be dirty. This can cause
> inconsistencies e.g. with read-only filesystems.
>
> Cc: Pavel Machek <pavel@....cz>
> Cc: Rafael J. Wysocki <rjw@...k.pl>
> Cc: Jiri Slaby <jslaby@...e.cz>
> Cc: linux-pm@...ts.linux-foundation.org
> Cc: linux-kernel@...r.kernel.org
> Cc: linux-s390@...r.kernel.org
> Signed-off-by: Martin Schwidefsky <schwidefsky@...ibm.com>
> ---
>
> arch/s390/kernel/swsusp_asm64.S | 3
> kernel/power/snapshot.c | 148 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 151 insertions(+)
>
> diff -urpN linux-2.6/arch/s390/kernel/swsusp_asm64.S linux-2.6-patched/arch/s390/kernel/swsusp_asm64.S
> --- linux-2.6/arch/s390/kernel/swsusp_asm64.S 2011-05-19 06:06:34.000000000 +0200
> +++ linux-2.6-patched/arch/s390/kernel/swsusp_asm64.S 2011-06-06 11:14:43.000000000 +0200
> @@ -138,11 +138,14 @@ swsusp_arch_resume:
> 0:
> lg %r2,8(%r1)
> lg %r4,0(%r1)
> + iske %r0,%r4
> lghi %r3,PAGE_SIZE
> lghi %r5,PAGE_SIZE
> 1:
> mvcle %r2,%r4,0
> jo 1b
> + lg %r2,8(%r1)
> + sske %r0,%r2
> lg %r1,16(%r1)
> ltgr %r1,%r1
> jnz 0b
> diff -urpN linux-2.6/kernel/power/snapshot.c linux-2.6-patched/kernel/power/snapshot.c
> --- linux-2.6/kernel/power/snapshot.c 2011-06-06 11:14:39.000000000 +0200
> +++ linux-2.6-patched/kernel/power/snapshot.c 2011-06-06 11:14:43.000000000 +0200
> @@ -1022,6 +1022,137 @@ static inline void copy_data_page(unsign
> }
> #endif /* CONFIG_HIGHMEM */
>
> +#ifdef CONFIG_S390
> +/*
> + * For s390 there is one additional byte associated with each page,
> + * the storage key. The key consists of the access-control bits
> + * (alias the key), the fetch-protection bit, the referenced bit
> + * and the change bit (alias dirty bit). Linux uses only the
> + * referenced bit and the change bit for pages in the page cache.
> + * Any modification of a page will set the change bit, any access
> + * sets the referenced bit. Overindication of the referenced bits
> + * after a hibernation cycle does not cause any harm but the
> + * overindication of the change bits would cause trouble.
> + * Therefore it is necessary to include the storage keys in the
> + * hibernation image. The storage keys are saved to the most
> + * significant byte of each page frame number in the list of pfns
> + * in the hibernation image.
> + */
> +
> +/*
> + * Key storage is allocated as a linked list of pages.
> + * The size of the keys array is (PAGE_SIZE - sizeof(long))
> + */
> +struct page_key_data {
> + struct page_key_data *next;
> + unsigned char data[];
> +};
> +
> +#define PAGE_KEY_DATA_SIZE (PAGE_SIZE - sizeof(struct page_key_data *))
> +
> +static struct page_key_data *page_key_data;
> +static struct page_key_data *page_key_rp, *page_key_wp;
> +static unsigned long page_key_rx, page_key_wx;
> +
> +/*
> + * For each page in the hibernation image one additional byte is
> + * stored in the most significant byte of the page frame number.
> + * On suspend no additional memory is required but on resume the
> + * keys need to be memorized until the page data has been restored.
> + * Only then can the storage keys be set to their old state.
> + */
> +static inline unsigned long page_key_additional_pages(unsigned long pages)
> +{
> + return DIV_ROUND_UP(pages, PAGE_KEY_DATA_SIZE);
> +}
> +
> +/*
> + * Free page_key_data list of arrays.
> + */
> +static void page_key_free(void)
> +{
> + struct page_key_data *pkd;
> +
> + while (page_key_data) {
> + pkd = page_key_data;
> + page_key_data = pkd->next;
> + free_page((unsigned long) pkd);
> + }
> +}
> +
> +/*
> + * Allocate page_key_data list of arrays with enough room to store
> + * one byte for each page in the hibernation image.
> + */
> +static int page_key_alloc(unsigned long pages)
> +{
> + struct page_key_data *pk;
> + unsigned long size;
> +
> + size = DIV_ROUND_UP(pages, PAGE_KEY_DATA_SIZE);
> + while (size--) {
> + pk = (struct page_key_data *) get_zeroed_page(GFP_KERNEL);
> + if (!pk) {
> + page_key_free();
> + return -ENOMEM;
> + }
> + pk->next = page_key_data;
> + page_key_data = pk;
> + }
> + page_key_rp = page_key_wp = page_key_data;
> + page_key_rx = page_key_wx = 0;
> + return 0;
> +}
> +
> +/*
> + * Save the storage key into the upper 8 bits of the page frame number.
> + */
> +static inline void page_key_read(unsigned long *pfn)
> +{
> + unsigned long addr;
> +
> + addr = (unsigned long) page_address(pfn_to_page(*pfn));
> + *(unsigned char *) pfn = (unsigned char) page_get_storage_key(addr);
> +}
> +
> +/*
> + * Extract the storage key from the upper 8 bits of the page frame number
> + * and store it in the page_key_data list of arrays.
> + */
> +static inline void page_key_memorize(unsigned long *pfn)
> +{
> + page_key_wp->data[page_key_wx] = *(unsigned char *) pfn;
> + *(unsigned char *) pfn = 0;
> + if (++page_key_wx < PAGE_KEY_DATA_SIZE)
> + return;
> + page_key_wp = page_key_wp->next;
> + page_key_wx = 0;
> +}
> +
> +/*
> + * Get the next key from the page_key_data list of arrays and set the
> + * storage key of the page referred by @address. If @address refers to
> + * a "safe" page the swsusp_arch_resume code will transfer the storage
> + * key from the buffer page to the original page.
> + */
> +static void page_key_write(void *address)
> +{
> + page_set_storage_key((unsigned long) address,
> + page_key_rp->data[page_key_rx], 0);
> + if (++page_key_rx >= PAGE_KEY_DATA_SIZE)
> + return;
> + page_key_rp = page_key_rp->next;
> + page_key_rx = 0;
> +}
> +#else
> +static unsigned long page_key_additional_pages(unsigned long pages) { return 0; }
> +static inline int page_key_alloc(unsigned long pages) { return 0; }
> +static inline void page_key_free(void) { }
> +static inline void page_key_read(unsigned long *pfn) { }
> +static inline void page_key_memorize(unsigned long *pfn) { }
> +static inline void page_key_write(void *address) { }
> +#endif
Having reconsidered things I think the code under the #ifdef above
should really go to arch/s390.
Now, for the purpose of exporting the headers I'd introduce
CONFIG_ARCH_SAVE_PAGE_KEYS and make S390 do
select ARCH_SAVE_PAGE_KEYS if HIBERNATION
and I'd put a #ifdef depending on that into include/linux/suspend.h.
Apart from this, I have only one complaint, which is that the kerneldoc
comments should follow the standard (the other comments in snapshot.c don't,
but that's a matter for a separate patch).
Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists