Patchwork [V2,4/6] ARM: Make PID_IN_CONTEXTIDR incompatible with PID_NS

login
register
mail settings
Submitter Adrien Vergé
Date Jan. 24, 2014, 4:40 p.m.
Message ID <1390581656-16372-5-git-send-email-adrienverge@gmail.com>
Download mbox | patch
Permalink /patch/313941/
State New
Headers show

Comments

Adrien Vergé - Jan. 24, 2014, 4:40 p.m.
When using namespaces, different processes can have the same PID.
It makes no sense to store a PID value in the Context ID register
to track a specific process, when others share the same value.

Consequently, PID_IN_CONTEXTIDR (which is used for tracing and
debugging processes) should not be compatible with PID_NS.

Signed-off-by: Adrien Vergé <adrienverge@gmail.com>
---
 arch/arm/Kconfig.debug   | 2 +-
 arch/arm64/Kconfig.debug | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)
Will Deacon - Jan. 24, 2014, 4:43 p.m.
On Fri, Jan 24, 2014 at 04:40:54PM +0000, Adrien Vergé wrote:
> When using namespaces, different processes can have the same PID.
> It makes no sense to store a PID value in the Context ID register
> to track a specific process, when others share the same value.
> 
> Consequently, PID_IN_CONTEXTIDR (which is used for tracing and
> debugging processes) should not be compatible with PID_NS.
> 
> Signed-off-by: Adrien Vergé <adrienverge@gmail.com>
> ---
>  arch/arm/Kconfig.debug   | 2 +-
>  arch/arm64/Kconfig.debug | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> index 5765abf..ed46748 100644
> --- a/arch/arm/Kconfig.debug
> +++ b/arch/arm/Kconfig.debug
> @@ -1143,7 +1143,7 @@ config ARM_KPROBES_TEST
>  
>  config PID_IN_CONTEXTIDR
>  	bool "Write the current PID to the CONTEXTIDR register"
> -	depends on CPU_COPY_V6
> +	depends on CPU_COPY_V6 && !PID_NS
>  	help
>  	  Enabling this option causes the kernel to write the current PID to
>  	  the PROCID field of the CONTEXTIDR register, at the expense of some
> diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
> index 835c559..06b2633b 100644
> --- a/arch/arm64/Kconfig.debug
> +++ b/arch/arm64/Kconfig.debug
> @@ -17,6 +17,7 @@ config EARLY_PRINTK
>  
>  config PID_IN_CONTEXTIDR
>  	bool "Write the current PID to the CONTEXTIDR register"
> +	depends on !PID_NS
>  	help
>  	  Enabling this option causes the kernel to write the current PID to
>  	  the CONTEXTIDR register, at the expense of some additional

Are you sure about this? The value we write is actually task_pid_nr, which I
believe to be globally unique.

Will
Adrien Vergé - Jan. 24, 2014, 5:16 p.m.
2014/1/24 Will Deacon <will.deacon@arm.com>:
> Are you sure about this? The value we write is actually task_pid_nr, which I
> believe to be globally unique.

You are right: the task_pid_nr is unique in the system. However when
using namespaces, the so called "PID" is the virtual number that
processes in different namespaces can share.

This PID is the one visible by user-space tasks, in particular
user-space tracers and debuggers. These programs would expect to find
the PID of the traced process in the Context ID reg, while it is not.
I think it is better to remove confusion by making PID_IN_CONTEXTIDR
and PID_NS incompatible.

What do you think?
Will Deacon - Jan. 24, 2014, 5:17 p.m.
On Fri, Jan 24, 2014 at 05:16:28PM +0000, Adrien Vergé wrote:
> 2014/1/24 Will Deacon <will.deacon@arm.com>:
> > Are you sure about this? The value we write is actually task_pid_nr, which I
> > believe to be globally unique.
> 
> You are right: the task_pid_nr is unique in the system. However when
> using namespaces, the so called "PID" is the virtual number that
> processes in different namespaces can share.
> 
> This PID is the one visible by user-space tasks, in particular
> user-space tracers and debuggers. These programs would expect to find
> the PID of the traced process in the Context ID reg, while it is not.
> I think it is better to remove confusion by making PID_IN_CONTEXTIDR
> and PID_NS incompatible.
> 
> What do you think?

I think I'd rather have the global ID than disable a potentially useful
feature, especially since this is likely to be consumed by external trace
tools as opposed to user-space tasks.

Will
Christopher Covington - Jan. 24, 2014, 5:52 p.m.
On 01/24/2014 12:17 PM, Will Deacon wrote:
> On Fri, Jan 24, 2014 at 05:16:28PM +0000, Adrien Vergé wrote:
>> 2014/1/24 Will Deacon <will.deacon@arm.com>:
>>> Are you sure about this? The value we write is actually task_pid_nr, which I
>>> believe to be globally unique.
>>
>> You are right: the task_pid_nr is unique in the system. However when
>> using namespaces, the so called "PID" is the virtual number that
>> processes in different namespaces can share.
>>
>> This PID is the one visible by user-space tasks, in particular
>> user-space tracers and debuggers. These programs would expect to find
>> the PID of the traced process in the Context ID reg, while it is not.
>> I think it is better to remove confusion by making PID_IN_CONTEXTIDR
>> and PID_NS incompatible.
>>
>> What do you think?
> 
> I think I'd rather have the global ID than disable a potentially useful
> feature, especially since this is likely to be consumed by external trace
> tools as opposed to user-space tasks.

We've discussed before that the ARM architecture doesn't say what should be
written to the CONTEXTIDR, so it's up to us to decide. Will has a use case
where the global PID is useful. Adrien's patches present a use case where I
think the virtual PID would be useful. I've done work in the past where
writing the process group ID was useful. Would it be reasonable to make what's
written to the CONTEXTIDR run-time configurable? If so, what would be the best
interface for configuring it?

Thanks,
Christopher
Christopher Covington - Jan. 24, 2014, 7:12 p.m.
On 01/24/2014 12:52 PM, Christopher Covington wrote:
> On 01/24/2014 12:17 PM, Will Deacon wrote:
>> On Fri, Jan 24, 2014 at 05:16:28PM +0000, Adrien Vergé wrote:
>>> 2014/1/24 Will Deacon <will.deacon@arm.com>:
>>>> Are you sure about this? The value we write is actually task_pid_nr, which I
>>>> believe to be globally unique.
>>>
>>> You are right: the task_pid_nr is unique in the system. However when
>>> using namespaces, the so called "PID" is the virtual number that
>>> processes in different namespaces can share.
>>>
>>> This PID is the one visible by user-space tasks, in particular
>>> user-space tracers and debuggers. These programs would expect to find
>>> the PID of the traced process in the Context ID reg, while it is not.
>>> I think it is better to remove confusion by making PID_IN_CONTEXTIDR
>>> and PID_NS incompatible.
>>>
>>> What do you think?
>>
>> I think I'd rather have the global ID than disable a potentially useful
>> feature, especially since this is likely to be consumed by external trace
>> tools as opposed to user-space tasks.
> 
> We've discussed before that the ARM architecture doesn't say what should be
> written to the CONTEXTIDR, so it's up to us to decide. Will has a use case
> where the global PID is useful. Adrien's patches present a use case where I
> think the virtual PID would be useful. I've done work in the past where
> writing the process group ID was useful. Would it be reasonable to make what's
> written to the CONTEXTIDR run-time configurable? If so, what would be the best
> interface for configuring it?

D'oh, I mixed things up. For ETM to work it can only use global PID's in the
CONTEXTIDR.

Christopher
Adrien Vergé - Jan. 24, 2014, 7:34 p.m.
2014/1/24 Will Deacon <will.deacon@arm.com>:
> I think I'd rather have the global ID than disable a potentially useful
> feature, especially since this is likely to be consumed by external trace
> tools as opposed to user-space tasks.

I understand.

2014/1/24 Christopher Covington <cov@codeaurora.org>:
> Would it be reasonable to make what's
> written to the CONTEXTIDR run-time configurable? If so, what would be the best
> interface for configuring it?

This is an interesting option.

An other option would be to keep the global PID in the Context ID
register, and rely on kernel support to translate virtual PID to
global PID when needed. Then, it would be possible to select a task to
trace via its PID, by asking the kernel to write its global ID to the
Context ID comparator.

Patch

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 5765abf..ed46748 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -1143,7 +1143,7 @@  config ARM_KPROBES_TEST
 
 config PID_IN_CONTEXTIDR
 	bool "Write the current PID to the CONTEXTIDR register"
-	depends on CPU_COPY_V6
+	depends on CPU_COPY_V6 && !PID_NS
 	help
 	  Enabling this option causes the kernel to write the current PID to
 	  the PROCID field of the CONTEXTIDR register, at the expense of some
diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
index 835c559..06b2633b 100644
--- a/arch/arm64/Kconfig.debug
+++ b/arch/arm64/Kconfig.debug
@@ -17,6 +17,7 @@  config EARLY_PRINTK
 
 config PID_IN_CONTEXTIDR
 	bool "Write the current PID to the CONTEXTIDR register"
+	depends on !PID_NS
 	help
 	  Enabling this option causes the kernel to write the current PID to
 	  the CONTEXTIDR register, at the expense of some additional