RE: page allocation/attributes question (i386/x86_64 specific)

[<Stuart_Hayes@Dell.com>]

randy_dunlap wrote:
> On Tue, 5 Jul 2005 15:02:26 -0500 Stuart_Hayes@Dell.com wrote:
> 
>> Hayes, Stuart wrote:
>>>> So, if I understand correctly what's going on in x86_64, your fix
>>>> wouldn't be applicable to i386.  In x86_64, every large page has a
>>>> correct "ref_prot" that is the normal setting for that page... but
>>>> in i386, the kernel text area does not--it should ideally be split
>>>> into small pages all the time if there are both kernel code & free
>>>> pages residing in the same 2M area.
>>>> 
>>>> Stuart
>>> 
>>> (This isn't a submission--I'm just posting this for comments.)
>>> 
>>> Right now, any large page that touches anywhere from PAGE_OFFSET to
>>> __init_end is initially set up as a large, executable page... but
>>> some of this area contains data & free pages.  The patch below adds
>>> a "cleanup_nx_in_kerneltext()" function, called at the end of
>>> free_initmem(), which changes these pages--except for the range from
>>> "_text" to "_etext"--to PAGE_KERNEL (i.e., non-executable).
>>> 
>>> This does result in two large pages being split up into small PTEs
>>> permanently, but all the non-code regions will be non-executable,
>>> and change_page_attr() will work correctly.
>>> 
>>> What do you think of this?  I have tested this on 2.6.12.
>>> 
>>> (I've attached the patch as a file, too, since my mail server can't
>>> be convinced to not wrap text.)
>>> 
>>> Stuart
>>> 
>> 
>> Andi--
>> 
>> I made another pass at this.  This does roughly the same thing, but
>> it doesn't create the new "change_page_attr_perm()" functions.  With
>> this patch, the change to init.c (cleanup_nx_in_kerneltext()) is
>> optional. I changed __change_page_attr() so that, if the page to be
>> changed is part of a large executable page, it splits the page up
>> *keeping the executability of the extra 511 pages*, and then marks
>> the new PTE page as reserved so that it won't be reverted.
>> 
>> So, basically, without the changes to init.c, the NX bits for data in
>> the first two big pages won't get fixed until someone calls
>> change_page_attr() on them.  If NX is disabled, these patches have
>> no functional effect at all. 
>> 
>> How does this look?
> 
> Look?  It has lots of bad line breaks and other style issues.
> 
> But I'll let Andi comment on the technical issues.
> 
>> -----
>> 
>> diff -purN linux-2.6.12grep/arch/i386/mm/init.c
>> linux-2.6.12/arch/i386/mm/init.c
>> --- linux-2.6.12grep/arch/i386/mm/init.c	2005-07-01
>> 15:09:27.000000000 -0500 +++
>> linux-2.6.12/arch/i386/mm/init.c	2005-07-05 14:32:57.000000000
-0500
>>  @@ -666,6 +666,28 @@ static int noinline do_test_wp_bit(void) 
>> return flag; } 
>> 
>> +/*
>> + * In kernel_physical_mapping_init(), any big pages that contained
>> kernel text area were + * set up as big executable pages.  This
>> function should be called + when the initmem
>> + * is freed, to correctly set up the executable & non-executable +
>> pages in this area.
>> + */
>> +static void cleanup_nx_in_kerneltext(void) {
>> +	unsigned long from, to;
>> +
>> +	if (!nx_enabled) return;
> 
> return; on separate line.
> 
>> +	from = PAGE_OFFSET;
>> +	to = (unsigned long)_text & PAGE_MASK;
>> +	for (; from<to; from += PAGE_SIZE)
> 	       from < to
> 
> (i.e., use the spacebar)
> 
>> +		change_page_attr(virt_to_page(from), 1, PAGE_KERNEL); +
>> +	from = ((unsigned long)_etext + PAGE_SIZE - 1) & PAGE_MASK;
>> +	to = ((unsigned long)__init_end + LARGE_PAGE_SIZE) &
>> LARGE_PAGE_MASK; +	for (; from<to; from += PAGE_SIZE)
> 
> add spaces:    from < to
> 
>> +		change_page_attr(virt_to_page(from), 1, PAGE_KERNEL); }
+
>>  void free_initmem(void)
>>  {
>>  	unsigned long addr;
> 
> 
> ---
> ~Randy

Thanks for the feedback.  I know it looks like crap with the line
wrapping, but I have no way of sending non-automatically-line-broken
email right now, which is why I attached the file in addition to the
inline text.  I apologize for that.

I'll correct those cosmetic issues, thanks.

Stuart
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/