Re: [PATCH] Kernel panic on IA-64 Linux with SELinux

Re: [PATCH] Kernel panic on IA-64 Linux with SELinux

[Kaigai Kohei]

James Morris wrote:
> Were you using a standard kernel, or was this a vendor or otherwise 
> patched kernel?  I know people have booted IA64 with SELinux enabled in 
> recent times with no problems.

Yes, I booted up both of the standard kernel (2.6.6) with both of the previous patches.
SELinux was configured to the Permissive-mode.
(I have not tried to boot up with the Enforcing-mode.)

I'm interested in the case you mentioned.
In my experience, kernel panic always occurs while booting up.
What is written in /etc/sysconfig/selinux?
--------
Kaigai Kohei, Linux Promotion Center, NEC
E-mail: kaigai@ak.jp.nec.com

[PATCH] Kernel panic on IA-64 Linux with SELinux

[@ 2004-06-09 4:36 UTC (permalink / raw)]

[-- Attachment #1: Type: text/plain, Size: 4549 bytes --]

Hello, everyone.

I observed kernel panic of linux-2.6.6 for IA-64, with SELinux.
(CONFIG_SECURITY=y and CONFIG_SECURITY_SELINUX=y)
I did a research about the problem, and found that there is an issue
with regard to the init_task initialization of IA-64.
I attached temporary workaround for the issue, but we should
come up with better solution.

[Overview]

While booing up the kernel with LSM/SELinux, init process tries to
execve() and one of the SELinux function is called to access init_task
(the parent of the init process)
Actually the LSM function tries to wake up tasks which is linked from
init_task, normally we expect nothing happens because init_task does
not have any linked tasks.
But because some members of init_task are initialized by wrong value,
due to the fact that IA-64 kernel image is now virtually mapped
from both region 5 and 7, it will lead to kernel panic.

I think this problem always happens when LSM/SELinux is enabled and
is IA-64 specific initialization issue.

[Detail]

The task_struct of cpu_idle (which is the parent of init process)
is named init_task and is defined in arch/ia64/kernel/init_task.c.
Init_task is initialized statically by INIT_TASK() macro at compile time.
The structure contains a wait_queue_head_t and a list_head in it.
Because the task_struct is linked to region 5 (0xa0....), the list_head
is also initialized to have region 5 addresses.

               |                       |
0xa0000000...  +=======================+
(region 5)     |                       |
               +-----------------------+
               |init_task              |
               |pid=0                  |
               |wait_chldexit.task_list|
               | +-------------------+ |
               | |next: 0xa000....   | |<--+ Initialized as empty list
               | +-------------------+ |   |  by referencing itself.
               | |prev: 0xa000....   | |<--+
               | +-------------------+ |
               |                       |
               +-----------------------+
               |                       |
              ===                     ===
               |                       |
0xe0000000...  +=======================+
(region 7)     |                       |
               +-----------------------+  <------ current of idle process
               |init_task              |
               |pid=0                  |
               |wait_chldexit.task_list|
               | +-------------------+ |
               | |next: 0xa000....   | |<--+ This list head is NOT EMPTY
               | +-------------------+ |   |  when accessed from region 7
               | |prev: 0xa000....   | |<--+
               | +-------------------+ |
               |                       |   <---+    +-----------------+
               +-----------------------+       |    | init process    |
               |                       |       |    | pid=1           |
               |                       |       +----- parent=init_task|
                                                    +-----------------+

As the current (r13) for the init_task is initialized in head.S to point
region 7 address (0xe0...), its child (init) will have region 7 address
for its parent address.

When the init process(PID=1) call execve(), LSM/SELinux hook function 
tries to access the wait_chldexit waiting queue of the cpu_idle.
Since this doubly linked list is broken by above reason,
kernel panic occurred.  Oops!

This problem came up to the surface for the first time by enabling SELinux.
Is it right referring to the parent of init?


[Solution]

Please find the attached patch to fix the problem.
The patch re-initializes current->wait_chldexit waitqueue, using
correct region 7 address in the head of setup_arch().

This solution is simple but sounds a bit ad hoc and I am seeking
better solution. I tried other 2 ways to fix the problem below,
but both of them failed.

1) I tried to move the init_task to region 7, by modifying linker
   script arch/ia64/kernel/vmlinux.lds.S.
   But I understand that the address of loading the kernel image is
   now be detemined dynamically (not compile time) and the INIT_TASK()
   macro still initializes the link list by wrong value :-<

2) We tried to set the current (r13) of cpu_idle to region 5 address.
   But it paniced and I could not grab the cause of the panic.
   Is there any reason we must put init_task in region 5?

Does anybody have better idea?

Best Regards, 
--
Kaigai Kohei, Linux Promotion Center, NEC
E-mail: kaigai@ak.jp.nec.com

[-- Attachment #2: ia64.selinux.nopanic.040607.patch --]
[-- Type: application/octet-stream, Size: 899 bytes --]

diff -rNU2 linux-2.6.6/arch/ia64/kernel/setup.c linux-2.6.6.selinux/arch/ia64/kernel/setup.c
--- linux-2.6.6/arch/ia64/kernel/setup.c	2004-05-10 11:32:01.000000000 +0900
+++ linux-2.6.6.selinux/arch/ia64/kernel/setup.c	2004-06-08 14:48:45.000000000 +0900
@@ -284,4 +284,14 @@
 setup_arch (char **cmdline_p)
 {
+	/*-----------------------------------------------------
+	 because task_struct of the parent of init(PID=0) was
+	 initialized statically,the members of list type
+	 in task_struct should be empty by self reference
+	 references 0xa000... But,since current is 0xe000...
+	 ,list_for_each() macro can not recognise this data
+	 as empty list. Thus,these member should be initialized
+	 dynamically. This problem was actualized in using SELinux.
+	 -----------------------------------------------------*/
+	init_waitqueue_head(&(current->wait_chldexit));
 	unw_init();
 

Re: [PATCH] Kernel panic on IA-64 Linux with SELinux

[Kazuto MIYOSHI]

Hi all, 

> 2) We tried to set the current (r13) of cpu_idle to region 5 address.
>    But it paniced and I could not grab the cause of the panic.
>    Is there any reason we must put init_task in region 5?

Please find the attached patch to make swapper to run on region 5.
It needs extra 2 instructions in ia64_switch_to() to stop mapping of
task stack if we are switching to idle, to avoid TLB entry duplication
caused by kernel text/data mapping and CURRENT_STACK mapping.

I believe swapper (init_task) should be consistent for its all members,
not only for wait_chldexit, and the patch will solve other potential
list_head/current in-consistency raised by swapper initialization,
though I am not so sure if it justifies extra instructions and
complication in context switch.

Best Regards,


--- linux-2.6.6/arch/ia64/kernel/head.S.org	2004-06-09 10:37:40.000000000 +0900
+++ linux-2.6.6/arch/ia64/kernel/head.S	2004-06-09 20:33:55.234192256 +0900
@@ -155,6 +155,10 @@
 #endif
 	;;
 	tpa r3=r2		// r3 = phys addr of task struct
+	;;
+	shr.u r16=r3,IA64_GRANULE_SHIFT
+(isBP)	br.cond.dpnt .load_current	// BP stack is on region 5 and no need to map it
+
 	// load mapping for stack (virtaddr in r2, physaddr in r3)
 	rsm psr.ic
 	movl r17=PAGE_KERNEL
@@ -166,7 +170,6 @@
 	dep r2=-1,r3,61,3	// IMVA of task
 	;;
 	mov r17=rr[r2]
-	shr.u r16=r3,IA64_GRANULE_SHIFT
 	;;
 	dep r17=0,r17,8,24
 	;;
@@ -181,6 +184,7 @@
 	srlz.d
   	;;
 
+.load_current:
 	// load the "current" pointer (r13) and ar.k6 with the current task
 	mov IA64_KR(CURRENT)=r2		// virtual address
 	mov IA64_KR(CURRENT_STACK)=r16


--- linux-2.6.6/arch/ia64/kernel/entry.S.org	2004-06-09 11:47:03.000000000 +0900
+++ linux-2.6.6/arch/ia64/kernel/entry.S	2004-06-09 14:28:21.000000000 +0900
@@ -178,6 +178,9 @@
 	DO_SAVE_SWITCH_STACK
 	.body
 
+	movl r25=init_task
+	;;
+	cmp.eq p7,p6=r25,in0
 	adds r22=IA64_TASK_THREAD_KSP_OFFSET,r13
 	mov r27=IA64_KR(CURRENT_STACK)
 	dep r20=0,in0,61,3		// physical address of "current"
@@ -189,7 +192,7 @@
 	/*
 	 * If we've already mapped this task's page, we can skip doing it again.
 	 */
-	cmp.eq p7,p6=r26,r27
+(p6)	cmp.eq p7,p6=r26,r27
 (p6)	br.cond.dpnt .map
 	;;
 .done:

Re: [PATCH] Kernel panic on IA-64 Linux with SELinux

[David Mosberger]

>>>>> On Wed, 09 Jun 2004 23:40:41 +0900, Kazuto MIYOSHI <miyoshi@linux.bs1.fc.nec.co.jp> said:

  >> 2) We tried to set the current (r13) of cpu_idle to region 5
  >> address.  But it paniced and I could not grab the cause of the
  >> panic.  Is there any reason we must put init_task in region 5?

  Kazuto> Please find the attached patch to make swapper to run on
  Kazuto> region 5.  It needs extra 2 instructions in ia64_switch_to()
  Kazuto> to stop mapping of task stack if we are switching to idle,
  Kazuto> to avoid TLB entry duplication caused by kernel text/data
  Kazuto> mapping and CURRENT_STACK mapping.

  Kazuto> I believe swapper (init_task) should be consistent for its
  Kazuto> all members, not only for wait_chldexit, and the patch will
  Kazuto> solve other potential list_head/current in-consistency
  Kazuto> raised by swapper initialization, though I am not so sure if
  Kazuto> it justifies extra instructions and complication in context
  Kazuto> switch.

That is in my opinion a worse patch compared to the reinitialization
that you proposed esterday.  Tony, what's your take on this problem?

	--david

RE: [PATCH] Kernel panic on IA-64 Linux with SELinux

[Luck, Tony]

>That is in my opinion a worse patch compared to the reinitialization
>that you proposed esterday.  Tony, what's your take on this problem?

I think it would be better to fix up init_task so that it appeared
to be in region 7 (that way it isn't a special case ... it just looks
like every other task).  BUT, I'm not sure how easy that is to do
without surgery to generic files (places that use "&init_task").

-Tony

RE: [PATCH] Kernel panic on IA-64 Linux with SELinux

[David Mosberger]

>>>>> On Wed, 9 Jun 2004 15:44:09 -0700, "Luck, Tony" <tony.luck@intel.com> said:

  Tony> I think it would be better to fix up init_task so that it
  Tony> appeared to be in region 7 (that way it isn't a special case
  Tony> ... it just looks like every other task).  BUT, I'm not sure
  Tony> how easy that is to do without surgery to generic files
  Tony> (places that use "&init_task").

For the particular case of the init_task, we could just fix it up
at boot time (as suggested by the earlier patch).

My bigger worry is that this will be an issue _whenever_ somebody
initializes a linked list with a static initializer.  There needs to
be a way to protect from this happening accidentially.  Without that,
I'm not sure the virtual remapping is really viable.

	--david

RE: [PATCH] Kernel panic on IA-64 Linux with SELinux

[Luck, Tony]

>For the particular case of the init_task, we could just fix it up
>at boot time (as suggested by the earlier patch).
>
>My bigger worry is that this will be an issue _whenever_ somebody
>initializes a linked list with a static initializer.  There needs to
>be a way to protect from this happening accidentially.  Without that,
>I'm not sure the virtual remapping is really viable.

This is only an issue if we view the same structure via both its
region 5 and region 7 aliases.  I think that init_task may currently
be a special case here ... in general arbitrary structures that users
declare and statically initialize with lists will only be referenced
via the region 5 alias ... so they will be safe.

-Tony

RE: [PATCH] Kernel panic on IA-64 Linux with SELinux

[David Mosberger]

>>>>> On Wed, 9 Jun 2004 16:35:48 -0700, "Luck, Tony" <tony.luck@intel.com> said:

  Tony> This is only an issue if we view the same structure via both
  Tony> its region 5 and region 7 aliases.  I think that init_task may
  Tony> currently be a special case here ... in general arbitrary
  Tony> structures that users declare and statically initialize with
  Tony> lists will only be referenced via the region 5 alias ... so
  Tony> they will be safe.

You're probably right.

We _could_ do something like this in init_task.c:

void
initialize_init_task (void)
{
	*current = (task_t) INIT_TASK((*current));
}

and then call this function from setup_arch().

The code does compile but doesn't seem to work.  I suspect there are
other lists that are inconsistent now.  Anybody want to track this
down?


	--david

RE: [PATCH] Kernel panic on IA-64 Linux with SELinux

[Luck, Tony]

>void
>initialize_init_task (void)
>{
>	*current = (task_t) INIT_TASK((*current));
>}
>
>and then call this function from setup_arch().
>
>The code does compile but doesn't seem to work.  I suspect there are
>other lists that are inconsistent now.  Anybody want to track this
>down?

Don't you need a "ia64_tpa()" call in there someplace to convert the
region 5 address to region 7?

-Tony

RE: [PATCH] Kernel panic on IA-64 Linux with SELinux

[David Mosberger]

>>>>> On Wed, 9 Jun 2004 22:19:28 -0700, "Luck, Tony" <tony.luck@intel.com> said:

  >> void initialize_init_task (void) { *current = (task_t)
  >> INIT_TASK((*current)); }

  >> and then call this function from setup_arch().

  >> The code does compile but doesn't seem to work.  I suspect there
  >> are other lists that are inconsistent now.  Anybody want to track
  >> this down?

  Tony> Don't you need a "ia64_tpa()" call in there someplace to
  Tony> convert the region 5 address to region 7?

"current" already contains the region 7 address.  Perhaps I'm misunderstanding
what you're saying, though.

	--david

RE: [PATCH] Kernel panic on IA-64 Linux with SELinux

[Luck, Tony]

>"current" already contains the region 7 address.  Perhaps I'm 
>misunderstanding what you're saying, though.

No, you aren't misunderstanding ... I'd just forgotten that
head.S converted from region 5 to region 7 before initializing
r13 (a.k.a. current).

I'll think about it more when I'm awake.

-Tony

Re: [PATCH] Kernel panic on IA-64 Linux with SELinux

[Kaigai Kohei]

Hello, everyone.

David Mosbeger wrote:

> You're probably right.
> 
> We _could_ do something like this in init_task.c:
> 
> void
> initialize_init_task (void)
> {
> *current = (task_t) INIT_TASK((*current));
> }
> 
> and then call this function from setup_arch().
> 
> The code does compile but doesn't seem to work.  I suspect there are
> other lists that are inconsistent now.  Anybody want to track this
> down?

I have applied this patch to 2.6.6 kernel, and system stall occurred
while booting up. The problem is in init_mount_tree().
The init_mount_tree() function uses do_each_thread() macro
defined as follows:

#define do_each_thread(g, t) \
        for (g = t = &init_task ; (g = t = next_task(g)) != &init_task ; ) do

The termination condition of this loop is g->tasks.next equals &init_task.
But g->tasks.next will not refer to &init_task forever,
because the init_task.task list_head was initialized as the data structure
in region 7 in spite of &init_task was in region 5.

There are similer problems
(The condition of the loop termination depends on &init_task).
- do_each_process() macro       ... in include/linux/sched.h
- is_devfsd_or_child() function ... in fs/devfs/base.c
- get_tgid_list() function      ... in fs/proc/base.c

There are several solutions against this problem.
(1) Initializing only wait_chldexit as my first patch.
(2) Moving current of swapper from region 7 to region 5 as
    Miyoshi's second patch.
(3) Replacing &init_task with task_t *p_init_task which refers to
    init_task in region 7, and refers to init_task directly on other archtectures.

(1) is easy but ad hoc. (3) needs to correct generic code.
(2) is consistent on &init_task is in region 5 and current(r13) for cpu_idle
 refers to region 5. But the number of steps in switch_to() macro will increase. 
 This overhead is not a big problem, I guess.

For reasons already stated I prefer (2). What do you think?

Best Regards, 
--------
Kaigai Kohei, Linux Promotion Center, NEC
E-mail: kaigai@ak.jp.nec.com

RE: [PATCH] Kernel panic on IA-64 Linux with SELinux

[Luck, Tony]

>> We _could_ do something like this in init_task.c:
>> 
>> void
>> initialize_init_task (void)
>> {
>> *current = (task_t) INIT_TASK((*current));
>> }
>> 
>> and then call this function from setup_arch().
>> 
>> The code does compile but doesn't seem to work.  I suspect there are
>> other lists that are inconsistent now.  Anybody want to track this
>> down?
>
>I have applied this patch to 2.6.6 kernel, and system stall occurred
>while booting up. The problem is in init_mount_tree().
>The init_mount_tree() function uses do_each_thread() macro
>defined as follows:
>
>#define do_each_thread(g, t) \
>        for (g = t = &init_task ; (g = t = next_task(g)) != 
>&init_task ; ) do
>
>The termination condition of this loop is g->tasks.next equals 
>&init_task.
>But g->tasks.next will not refer to &init_task forever,
>because the init_task.task list_head was initialized as the 
>data structure
>in region 7 in spite of &init_task was in region 5.

I managed to take a look at what David's code actually did this
afternoon.  It does manage to fix up all the list_head structures,
but as noted above it messes up the list of all tasks since we
converted to region 7, and now we'll never find a link with the
region 5 address "&init_task".

I also found that the "lock_depth" entry is messed up (because
INIT_TASK resets it to "-1", undoing the "lock_kernel()" call
that start_kernel() did before it called setup_arch().

We could go for a slightly uglier version of initialize_init_task()
that just fixes the list_head elements ... there are only
six of them ... so in one respect this isn't too ugly, but it
would require maintenance every time someone added/removed
a new list from the task structure.

-Tony

Re: [PATCH] Kernel panic on IA-64 Linux with SELinux

[David Mosberger]

Hi Kaigai,

Thanks for looking into this issue.

>>>>> On Tue, 15 Jun 2004 11:28:14 +0900, "Kaigai Kohei" <kaigai@ak.jp.nec.com> said:

  Kaigai> I have applied this patch to 2.6.6 kernel, and system stall
  Kaigai> occurred while booting up. The problem is in
  Kaigai> init_mount_tree().  The init_mount_tree() function uses
  Kaigai> do_each_thread() macro defined as follows:

  Kaigai> #define do_each_thread(g, t) \
  Kaigai>   for (g = t = &init_task ; (g = t = next_task(g)) != &init_task ; ) do

Yeah, that'd do it.

  Kaigai> There are several solutions against this problem.

  Kaigai> (1) Initializing only wait_chldexit as my first patch.

  Kaigai> (2) Moving current of swapper from region 7 to region 5 as
  Kaigai> Miyoshi's second patch.

  Kaigai> (3) Replacing &init_task with task_t *p_init_task which
  Kaigai> refers to init_task in region 7, and refers to init_task
  Kaigai> directly on other archtectures.

  Kaigai> (1) is easy but ad hoc. (3) needs to correct generic code.
  Kaigai> (2) is consistent on &init_task is in region 5 and
  Kaigai> current(r13) for cpu_idle refers to region 5. But the number
  Kaigai> of steps in switch_to() macro will increase.  This overhead
  Kaigai> is not a big problem, I guess.

  Kaigai> For reasons already stated I prefer (2). What do you think?

I agree, (2) seems to best solution so far.  It maintains the
invariant that static data will be in region 5.  The extra
instructions should be pretty much free since it should be possible to
schedule them into existing stalls.

Can you resend the tested patch with proper changelog and "Signed-off-by"
trailer?

Thanks,

	--david

RE: [PATCH] Kernel panic on IA-64 Linux with SELinux

[Luck, Tony]

>There are similer problems
>(The condition of the loop termination depends on &init_task).
>- do_each_process() macro       ... in include/linux/sched.h
>- is_devfsd_or_child() function ... in fs/devfs/base.c
>- get_tgid_list() function      ... in fs/proc/base.c

Grepping through the kernel shows nine places where
generic code is using "&init_task".  (Way up from the
one that existed when the kernel relocation patch was
first dreamed of :-( )  An no guarantee that people won't
add more.

Your option 2 is looking like the best choice.

-Tony

RE: [PATCH] Kernel panic on IA-64 Linux with SELinux

[David Mosberger]

>>>>> On Mon, 14 Jun 2004 21:17:34 -0700, "Luck, Tony" <tony.luck@intel.com> said:

  Tony> We could go for a slightly uglier version of
  Tony> initialize_init_task() that just fixes the list_head elements
  Tony> ... there are only six of them ... so in one respect this
  Tony> isn't too ugly, but it would require maintenance every time
  Tony> someone added/removed a new list from the task structure.

I'm worried that would only defer the pain.  As long as we keep in
mind that static data resides in region 5, we should be OK with
Kaigai's second solution ("current" points to region 5 address for
swapper task).

	--david

Re: [PATCH] Kernel panic on IA-64 Linux with SELinux

[Kaigai Kohei]

[-- Attachment #1: Type: text/plain, Size: 2351 bytes --]

Dear David

The attached patch moves the current of cpu_idle from region 7 to region 5.

The init_task can be viewed from either region 5 or 7.
But the current(r13) of cpu_idle was a region 7 address while &init_task
referes to a region 5 address. Thus, some bad effects occurred.
(This problem came to the front first time by enabling SELinux.)


Signed-off-by: Kazuto MIYOSHI <k-miyoshi@cb.jp.nec.com>
Signed-off-by: Takayoshi Kochi <t-kochi@bq.jp.nec.com>
Signed-off-by: Kaigai Kohei <kaigai@ak.jp.nec.com>

Thanks, so much.
--------
Kaigai Kohei, Linux Promotion Center, NEC
E-mail: kaigai@ak.jp.nec.com


> Hi Kaigai,
> 
> Thanks for looking into this issue.
> 
> >>>>> On Tue, 15 Jun 2004 11:28:14 +0900, "Kaigai Kohei" <kaigai@ak.jp.nec.com> said:
> 
>   Kaigai> I have applied this patch to 2.6.6 kernel, and system stall
>   Kaigai> occurred while booting up. The problem is in
>   Kaigai> init_mount_tree().  The init_mount_tree() function uses
>   Kaigai> do_each_thread() macro defined as follows:
> 
>   Kaigai> #define do_each_thread(g, t) \
>   Kaigai>   for (g = t = &init_task ; (g = t = next_task(g)) != &init_task ; ) do
> 
> Yeah, that'd do it.
> 
>   Kaigai> There are several solutions against this problem.
> 
>   Kaigai> (1) Initializing only wait_chldexit as my first patch.
> 
>   Kaigai> (2) Moving current of swapper from region 7 to region 5 as
>   Kaigai> Miyoshi's second patch.
> 
>   Kaigai> (3) Replacing &init_task with task_t *p_init_task which
>   Kaigai> refers to init_task in region 7, and refers to init_task
>   Kaigai> directly on other archtectures.
> 
>   Kaigai> (1) is easy but ad hoc. (3) needs to correct generic code.
>   Kaigai> (2) is consistent on &init_task is in region 5 and
>   Kaigai> current(r13) for cpu_idle refers to region 5. But the number
>   Kaigai> of steps in switch_to() macro will increase.  This overhead
>   Kaigai> is not a big problem, I guess.
> 
>   Kaigai> For reasons already stated I prefer (2). What do you think?
> 
> I agree, (2) seems to best solution so far.  It maintains the
> invariant that static data will be in region 5.  The extra
> instructions should be pretty much free since it should be possible to
> schedule them into existing stalls.
> 
> Can you resend the tested patch with proper changelog and "Signed-off-by"
> trailer?
> 
> Thanks,
> 
> --david
> 

[-- Attachment #2: migrate.init_task.2.6.7-rc3.patch --]
[-- Type: application/octet-stream, Size: 1156 bytes --]

diff -rNU2 linux-2.6.7-rc3/arch/ia64/kernel/entry.S linux-2.6.7-rc3.selinux/arch/ia64/kernel/entry.S
--- linux-2.6.7-rc3/arch/ia64/kernel/entry.S	2004-06-08 04:14:02.000000000 +0900
+++ linux-2.6.7-rc3.selinux/arch/ia64/kernel/entry.S	2004-06-15 15:19:24.925797780 +0900
@@ -178,5 +178,8 @@
 	DO_SAVE_SWITCH_STACK
 	.body
-
+	
+	movl r22=init_task;;
+	cmp.eq p7,p6=r22,in0
+	
 	adds r22=IA64_TASK_THREAD_KSP_OFFSET,r13
 	mov r27=IA64_KR(CURRENT_STACK)
@@ -190,5 +193,5 @@
 	 * If we've already mapped this task's page, we can skip doing it again.
 	 */
-	cmp.eq p7,p6=r26,r27
+(p6)	cmp.eq p7,p6=r26,r27
 (p6)	br.cond.dpnt .map
 	;;
diff -rNU2 linux-2.6.7-rc3/arch/ia64/kernel/head.S linux-2.6.7-rc3.selinux/arch/ia64/kernel/head.S
--- linux-2.6.7-rc3/arch/ia64/kernel/head.S	2004-06-08 04:14:42.000000000 +0900
+++ linux-2.6.7-rc3.selinux/arch/ia64/kernel/head.S	2004-06-15 15:15:57.434480607 +0900
@@ -182,4 +182,6 @@
 
 	// load the "current" pointer (r13) and ar.k6 with the current task
+	(isBP) movl r2=init_task	// migrate to region 5
+	;; 
 	mov IA64_KR(CURRENT)=r2		// virtual address
 	mov IA64_KR(CURRENT_STACK)=r16

Re: [PATCH] Kernel panic on IA-64 Linux with SELinux

[Kaigai Kohei]

[-- Attachment #1: Type: text/plain, Size: 3213 bytes --]

I'm sorry the patch attached with the previous mail was wrong.
Please ignore it. orz

The patch attached with this mail is the latest version.

------------------------------------
Dear David

The attached patch moves the current of cpu_idle from region 7 to region 5.

The init_task can be viewed from either region 5 or 7.
But the current(r13) of cpu_idle was a region 7 address while &init_task
referes to a region 5 address. Thus, some bad effects occurred.
(This problem came to the front first time by enabling SELinux.)


Signed-off-by: Kazuto MIYOSHI <k-miyoshi@cb.jp.nec.com>
Signed-off-by: Takayoshi Kochi <t-kochi@bq.jp.nec.com>
Signed-off-by: Kaigai Kohei <kaigai@ak.jp.nec.com>
------------------------------------

> Dear David
> 
> The attached patch moves the current of cpu_idle from region 7 to region 5.
> 
> The init_task can be viewed from either region 5 or 7.
> But the current(r13) of cpu_idle was a region 7 address while &init_task
> referes to a region 5 address. Thus, some bad effects occurred.
> (This problem came to the front first time by enabling SELinux.)
> 
> 
> Signed-off-by: Kazuto MIYOSHI <k-miyoshi@cb.jp.nec.com>
> Signed-off-by: Takayoshi Kochi <t-kochi@bq.jp.nec.com>
> Signed-off-by: Kaigai Kohei <kaigai@ak.jp.nec.com>
> 
> Thanks, so much.
> --------
> Kaigai Kohei, Linux Promotion Center, NEC
> E-mail: kaigai@ak.jp.nec.com
> 
> 
> > Hi Kaigai,
> > 
> > Thanks for looking into this issue.
> > 
> > >>>>> On Tue, 15 Jun 2004 11:28:14 +0900, "Kaigai Kohei" <kaigai@ak.jp.nec.com> said:
> > 
> >   Kaigai> I have applied this patch to 2.6.6 kernel, and system stall
> >   Kaigai> occurred while booting up. The problem is in
> >   Kaigai> init_mount_tree().  The init_mount_tree() function uses
> >   Kaigai> do_each_thread() macro defined as follows:
> > 
> >   Kaigai> #define do_each_thread(g, t) \
> >   Kaigai>   for (g = t = &init_task ; (g = t = next_task(g)) != &init_task ; ) do
> > 
> > Yeah, that'd do it.
> > 
> >   Kaigai> There are several solutions against this problem.
> > 
> >   Kaigai> (1) Initializing only wait_chldexit as my first patch.
> > 
> >   Kaigai> (2) Moving current of swapper from region 7 to region 5 as
> >   Kaigai> Miyoshi's second patch.
> > 
> >   Kaigai> (3) Replacing &init_task with task_t *p_init_task which
> >   Kaigai> refers to init_task in region 7, and refers to init_task
> >   Kaigai> directly on other archtectures.
> > 
> >   Kaigai> (1) is easy but ad hoc. (3) needs to correct generic code.
> >   Kaigai> (2) is consistent on &init_task is in region 5 and
> >   Kaigai> current(r13) for cpu_idle refers to region 5. But the number
> >   Kaigai> of steps in switch_to() macro will increase.  This overhead
> >   Kaigai> is not a big problem, I guess.
> > 
> >   Kaigai> For reasons already stated I prefer (2). What do you think?
> > 
> > I agree, (2) seems to best solution so far.  It maintains the
> > invariant that static data will be in region 5.  The extra
> > instructions should be pretty much free since it should be possible to
> > schedule them into existing stalls.
> > 
> > Can you resend the tested patch with proper changelog and "Signed-off-by"
> > trailer?
> > 
> > Thanks,
> > 
> > --david
> >

[-- Attachment #2: migrate.init_task.2.6.7-rc3.040615.patch --]
[-- Type: application/octet-stream, Size: 1479 bytes --]

diff -rNU2 linux-2.6.7-rc3/arch/ia64/kernel/entry.S linux-2.6.7-rc3.selinux/arch/ia64/kernel/entry.S
--- linux-2.6.7-rc3/arch/ia64/kernel/entry.S	2004-06-08 04:14:02.000000000 +0900
+++ linux-2.6.7-rc3.selinux/arch/ia64/kernel/entry.S	2004-06-15 19:29:47.981929427 +0900
@@ -178,5 +178,9 @@
 	DO_SAVE_SWITCH_STACK
 	.body
-
+	
+	movl r25=init_task
+	;;
+	cmp.eq p7,p6=r25,in0
+	
 	adds r22=IA64_TASK_THREAD_KSP_OFFSET,r13
 	mov r27=IA64_KR(CURRENT_STACK)
@@ -190,5 +194,5 @@
 	 * If we've already mapped this task's page, we can skip doing it again.
 	 */
-	cmp.eq p7,p6=r26,r27
+(p6)	cmp.eq p7,p6=r26,r27
 (p6)	br.cond.dpnt .map
 	;;
diff -rNU2 linux-2.6.7-rc3/arch/ia64/kernel/head.S linux-2.6.7-rc3.selinux/arch/ia64/kernel/head.S
--- linux-2.6.7-rc3/arch/ia64/kernel/head.S	2004-06-08 04:14:42.000000000 +0900
+++ linux-2.6.7-rc3.selinux/arch/ia64/kernel/head.S	2004-06-15 19:26:26.912398427 +0900
@@ -155,4 +155,7 @@
 	;;
 	tpa r3=r2		// r3 == phys addr of task struct
+	;;
+	shr.u r16=r3,IA64_GRANULE_SHIFT
+	(isBP) br.cond.dpnt .load_current // BP stack is on region 5 and no need to map it
 	// load mapping for stack (virtaddr in r2, physaddr in r3)
 	rsm psr.ic
@@ -166,5 +169,4 @@
 	;;
 	mov r17=rr[r2]
-	shr.u r16=r3,IA64_GRANULE_SHIFT
 	;;
 	dep r17=0,r17,8,24
@@ -181,4 +183,5 @@
   	;;
 
+.load_current:
 	// load the "current" pointer (r13) and ar.k6 with the current task
 	mov IA64_KR(CURRENT)=r2		// virtual address

Re: [PATCH] Kernel panic on IA-64 Linux with SELinux

[David Mosberger]

>>>>> On Tue, 15 Jun 2004 21:52:26 +0900, "Kaigai Kohei" <kaigai@ak.jp.nec.com> said:

  Kaigai> I'm sorry the patch attached with the previous mail was
  Kaigai> wrong.  Please ignore it. orz

  Kaigai> The patch attached with this mail is the latest version.

OK, I check this in now, with a small change to the ia64_switch_to()
portion, so that the instructions get packed better.

Thanks for tracking this down and fixing it!

	--david