[PATCH] Read THREAD_CPUTIME clock from other processes.

[PATCH] Read THREAD_CPUTIME clock from other processes.

[Dario Faggioli]

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

Trying to read CLOCK_THREAD_CPUTIME_ID of a thread from outside
the process that spawned it with this code:

        if (clock_getcpuclockid(tid, &clockid) != 0) {
                perror("clock_getcpuclockid");
                exit(EXIT_FAILURE);
        }

results in this:
  ### Testing tid 24207: CPU-time clock for PID 24207 is 1.132371729 seconds
  ### Testing tid 24209: clock_getcpuclockid: Success

OTH, if full-fledged processes are involved, the behaviour is this:
  ### Testing tid 24218: CPU-time clock for PID 24218 is 0.001059305 seconds
  ### Testing tid 24220: CPU-time clock for PID 24220 is 1.044057391 seconds

Test programs available here: http://gitorious.org/clockid.

This is because clock_getcpuclockid forbids accessing thread
specific CPU-time clocks from outside the thread group. This is
not requested (e.g., by POSIX) to be like this, or at least no
indication that such operation should fail can be found in
`man clock_getcpuclockid' and alike.

However, having such capability could be useful, if you want
to monitor the execution of a bunch of thread from some kind of
"manager" which might not be part of the same process. A typical
example that could benefit from this could be the JACK graph-manager.

Therefore, this patch removes such limitation and enables the
following behaviour, for the threaded and process-based case,
respectively:

  ### Testing tid 24704: CPU-time clock for PID 24704 is 1.049570008 seconds
  ### Testing tid 24706: CPU-time clock for PID 24706 is 1.028650801seconds

  ### Testing tid 24715: CPU-time clock for PID 24715 is 0.000957685 seconds
  ### Testing tid 24717: CPU-time clock for PID 24717 is 1.045351509 seconds

Signed-off-by: Dario Faggioli <raistlin@linux.it>
---
 kernel/posix-cpu-timers.c |   23 ++++++++++++-----------
 1 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
index 05bb717..b0ed8cf 100644
--- a/kernel/posix-cpu-timers.c
+++ b/kernel/posix-cpu-timers.c
@@ -39,10 +39,8 @@ static int check_clock(const clockid_t which_clock)
 
 	rcu_read_lock();
 	p = find_task_by_vpid(pid);
-	if (!p || !(CPUCLOCK_PERTHREAD(which_clock) ?
-		   same_thread_group(p, current) : has_group_leader_pid(p))) {
+	if (!p)
 		error = -EINVAL;
-	}
 	rcu_read_unlock();
 
 	return error;
@@ -349,18 +347,21 @@ int posix_cpu_clock_get(const clockid_t which_clock, struct timespec *tp)
 		rcu_read_lock();
 		p = find_task_by_vpid(pid);
 		if (p) {
-			if (CPUCLOCK_PERTHREAD(which_clock)) {
-				if (same_thread_group(p, current)) {
-					error = cpu_clock_sample(which_clock,
-								 p, &rtn);
-				}
+
+			if (CPUCLOCK_PERTHREAD(which_clock) &&
+			    same_thread_group(p, current)) {
+				error = cpu_clock_sample(which_clock,
+							 p, &rtn);
 			} else {
 				read_lock(&tasklist_lock);
-				if (thread_group_leader(p) && p->sighand) {
+				if (!CPUCLOCK_PERTHREAD(which_clock) &&
+				    thread_group_leader(p) && p->sighand)
 					error =
 					    cpu_clock_sample_group(which_clock,
-							           p, &rtn);
-				}
+								   p, &rtn);
+				else
+					error = cpu_clock_sample(which_clock,
+								 p, &rtn);
 				read_unlock(&tasklist_lock);
 			}
 		}
-- 
1.7.2.3


-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
----------------------------------------------------------------------
Dario Faggioli, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa  (Italy)

http://retis.sssup.it/people/faggioli -- dario.faggioli@jabber.org

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] Read THREAD_CPUTIME clock from other processes.

[Oleg Nesterov]

On 12/23, Dario Faggioli wrote:
>
> Trying to read CLOCK_THREAD_CPUTIME_ID of a thread from outside
> the process that spawned it with this code:
>
>         if (clock_getcpuclockid(tid, &clockid) != 0) {
>                 perror("clock_getcpuclockid");
>                 exit(EXIT_FAILURE);
>         }
>
> results in this:
>   ### Testing tid 24207: CPU-time clock for PID 24207 is 1.132371729 seconds
>   ### Testing tid 24209: clock_getcpuclockid: Success
>
> OTH, if full-fledged processes are involved, the behaviour is this:
>   ### Testing tid 24218: CPU-time clock for PID 24218 is 0.001059305 seconds
>   ### Testing tid 24220: CPU-time clock for PID 24220 is 1.044057391 seconds
>
> Test programs available here: http://gitorious.org/clockid.
>
> This is because clock_getcpuclockid forbids accessing thread
> specific CPU-time clocks from outside the thread group. This is
> not requested (e.g., by POSIX) to be like this, or at least no
> indication that such operation should fail can be found in
> `man clock_getcpuclockid' and alike.
>
> However, having such capability could be useful, if you want
> to monitor the execution of a bunch of thread from some kind of
> "manager" which might not be part of the same process. A typical
> example that could benefit from this could be the JACK graph-manager.
>
> Therefore, this patch removes such limitation and enables the
> following behaviour, for the threaded and process-based case,
> respectively:

Can't comment, I never understood this.


A couple of nits on the patch itself,

> --- a/kernel/posix-cpu-timers.c
> +++ b/kernel/posix-cpu-timers.c
> @@ -39,10 +39,8 @@ static int check_clock(const clockid_t which_clock)
>
>  	rcu_read_lock();
>  	p = find_task_by_vpid(pid);
> -	if (!p || !(CPUCLOCK_PERTHREAD(which_clock) ?
> -		   same_thread_group(p, current) : has_group_leader_pid(p))) {
> +	if (!p)
>  		error = -EINVAL;
> -	}

This changes the behaviour of sys_clock_settime(). Probably doesn't
matter since it does nothing, but perhaps !CPUCLOCK_PERTHREAD &&
!group_leader should result in -EINAVL as before.

> @@ -349,18 +347,21 @@ int posix_cpu_clock_get(const clockid_t which_clock, struct timespec *tp)
>  		rcu_read_lock();
>  		p = find_task_by_vpid(pid);
>  		if (p) {
> -			if (CPUCLOCK_PERTHREAD(which_clock)) {
> -				if (same_thread_group(p, current)) {
> -					error = cpu_clock_sample(which_clock,
> -								 p, &rtn);
> -				}
> +
> +			if (CPUCLOCK_PERTHREAD(which_clock) &&
> +			    same_thread_group(p, current)) {
> +				error = cpu_clock_sample(which_clock,
> +							 p, &rtn);
>  			} else {
>  				read_lock(&tasklist_lock);
> -				if (thread_group_leader(p) && p->sighand) {
> +				if (!CPUCLOCK_PERTHREAD(which_clock) &&
> +				    thread_group_leader(p) && p->sighand)
>  					error =
>  					    cpu_clock_sample_group(which_clock,
> -							           p, &rtn);
> -				}
> +								   p, &rtn);
> +				else
> +					error = cpu_clock_sample(which_clock,
> +								 p, &rtn);
>  				read_unlock(&tasklist_lock);

Can't understand... why did you duplicate cpu_clock_sample() ?

IOW, it seems to me you could simply kill the
"if (same_thread_group(p, current)) {" line with the same efect, no?

Oleg.

Re: [PATCH] Read THREAD_CPUTIME clock from other processes.

[Dario Faggioli]

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

On Thu, 2010-12-23 at 17:44 +0100, Oleg Nesterov wrote: 
> > Therefore, this patch removes such limitation and enables the
> > following behaviour, for the threaded and process-based case,
> > respectively:
> 
> Can't comment, I never understood this.
> 
If I can ask... What's that you never understood? Why the limitation is
there? Or something else?

> >  	rcu_read_lock();
> >  	p = find_task_by_vpid(pid);
> > -	if (!p || !(CPUCLOCK_PERTHREAD(which_clock) ?
> > -		   same_thread_group(p, current) : has_group_leader_pid(p))) {
> > +	if (!p)
> >  		error = -EINVAL;
> > -	}
> 
> This changes the behaviour of sys_clock_settime(). Probably doesn't
> matter since it does nothing, but perhaps !CPUCLOCK_PERTHREAD &&
> !group_leader should result in -EINAVL as before.
> 
Oops, sure, you're right, I can fix this. :-)

> > @@ -349,18 +347,21 @@ int posix_cpu_clock_get(const clockid_t which_clock, struct timespec *tp)
> >  		rcu_read_lock();
> >  		p = find_task_by_vpid(pid);
> >  		if (p) {
> > -			if (CPUCLOCK_PERTHREAD(which_clock)) {
> > -				if (same_thread_group(p, current)) {
> > -					error = cpu_clock_sample(which_clock,
> > -								 p, &rtn);
> > -				}
> > +
> > +			if (CPUCLOCK_PERTHREAD(which_clock) &&
> > +			    same_thread_group(p, current)) {
> > +				error = cpu_clock_sample(which_clock,
> > +							 p, &rtn);
> >  			} else {
> >  				read_lock(&tasklist_lock);
> > -				if (thread_group_leader(p) && p->sighand) {
> > +				if (!CPUCLOCK_PERTHREAD(which_clock) &&
> > +				    thread_group_leader(p) && p->sighand)
> >  					error =
> >  					    cpu_clock_sample_group(which_clock,
> > -							           p, &rtn);
> > -				}
> > +								   p, &rtn);
> > +				else
> > +					error = cpu_clock_sample(which_clock,
> > +								 p, &rtn);
> >  				read_unlock(&tasklist_lock);
> 
> Can't understand... why did you duplicate cpu_clock_sample() ?
> 
> IOW, it seems to me you could simply kill the
> "if (same_thread_group(p, current)) {" line with the same efect, no?
> 
Well, yes, but looking at the original code I thought that in the !
same_thread_group() case I might need the tasklist_lock...

Am I wrong? Is it there just because of cpu_clock_sample_group()?

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
----------------------------------------------------------------------
Dario Faggioli, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa  (Italy)

http://retis.sssup.it/people/faggioli -- dario.faggioli@jabber.org

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] Read THREAD_CPUTIME clock from other processes.

[Oleg Nesterov]

On 12/23, Dario Faggioli wrote:
>
> On Thu, 2010-12-23 at 17:44 +0100, Oleg Nesterov wrote:
> > > Therefore, this patch removes such limitation and enables the
> > > following behaviour, for the threaded and process-based case,
> > > respectively:
> >
> > Can't comment, I never understood this.
> >
> If I can ask... What's that you never understood? Why the limitation is
> there?

Yes. IOW, I agree it looks strange, clock_gettime() can sample the
whole group but not a single thread.

> > > @@ -349,18 +347,21 @@ int posix_cpu_clock_get(const clockid_t which_clock, struct timespec *tp)
> > >  		rcu_read_lock();
> > >  		p = find_task_by_vpid(pid);
> > >  		if (p) {
> > > -			if (CPUCLOCK_PERTHREAD(which_clock)) {
> > > -				if (same_thread_group(p, current)) {
> > > -					error = cpu_clock_sample(which_clock,
> > > -								 p, &rtn);
> > > -				}
> > > +
> > > +			if (CPUCLOCK_PERTHREAD(which_clock) &&
> > > +			    same_thread_group(p, current)) {
> > > +				error = cpu_clock_sample(which_clock,
> > > +							 p, &rtn);
> > >  			} else {
> > >  				read_lock(&tasklist_lock);
> > > -				if (thread_group_leader(p) && p->sighand) {
> > > +				if (!CPUCLOCK_PERTHREAD(which_clock) &&
> > > +				    thread_group_leader(p) && p->sighand)
> > >  					error =
> > >  					    cpu_clock_sample_group(which_clock,
> > > -							           p, &rtn);
> > > -				}
> > > +								   p, &rtn);
> > > +				else
> > > +					error = cpu_clock_sample(which_clock,
> > > +								 p, &rtn);
> > >  				read_unlock(&tasklist_lock);
> >
> > Can't understand... why did you duplicate cpu_clock_sample() ?
> >
> > IOW, it seems to me you could simply kill the
> > "if (same_thread_group(p, current)) {" line with the same efect, no?
> >
> Well, yes, but looking at the original code I thought that in the !
> same_thread_group() case I might need the tasklist_lock...
>
> Am I wrong? Is it there just because of cpu_clock_sample_group()?

Yes, it is because of _group (we are going to sample all sub-threads),
not because of !same_thread_group().

Oh. In fact we should remove this tasklist, but this is another story.

Oleg.

Re: [PATCH] Read THREAD_CPUTIME clock from other processes.

[Dario Faggioli]

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

On Thu, 2010-12-23 at 18:38 +0100, Dario Faggioli wrote: 
> > >  	rcu_read_lock();
> > >  	p = find_task_by_vpid(pid);
> > > -	if (!p || !(CPUCLOCK_PERTHREAD(which_clock) ?
> > > -		   same_thread_group(p, current) : has_group_leader_pid(p))) {
> > > +	if (!p)
> > >  		error = -EINVAL;
> > > -	}
> > 
> > This changes the behaviour of sys_clock_settime(). Probably doesn't
> > matter since it does nothing, but perhaps !CPUCLOCK_PERTHREAD &&
> > !group_leader should result in -EINAVL as before.
> > 
> Oops, sure, you're right, I can fix this. :-)
> 
Mmm... This is trickier than I remembered expected. Apparently, glibc
always gives us a !CPUCLOCK_PERTHREAD() clockid, since it uses
MAKE_PROCESS_CLOCK() when clock_cpugetclockid is called.

Therefore, the take two of this thing will be a little different from
this first posting...

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
----------------------------------------------------------------------
Dario Faggioli, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa  (Italy)

http://retis.sssup.it/people/faggioli -- dario.faggioli@jabber.org


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] Read THREAD_CPUTIME clock from other processes.

[Randy Dunlap]

On Thu, 23 Dec 2010 17:21:43 +0100 Dario Faggioli wrote:

> Trying to read CLOCK_THREAD_CPUTIME_ID of a thread from outside
> the process that spawned it with this code:
> 
>         if (clock_getcpuclockid(tid, &clockid) != 0) {
>                 perror("clock_getcpuclockid");
>                 exit(EXIT_FAILURE);
>         }
> 
> results in this:
>   ### Testing tid 24207: CPU-time clock for PID 24207 is 1.132371729 seconds
>   ### Testing tid 24209: clock_getcpuclockid: Success
> 
> OTH, if full-fledged processes are involved, the behaviour is this:
>   ### Testing tid 24218: CPU-time clock for PID 24218 is 0.001059305 seconds
>   ### Testing tid 24220: CPU-time clock for PID 24220 is 1.044057391 seconds
> 
> Test programs available here: http://gitorious.org/clockid.


"DNS service for this domain has expired with DNS Made Easy"  :(

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
desserts:  http://www.xenotime.net/linux/recipes/

Re: [PATCH] Read THREAD_CPUTIME clock from other processes.

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 1449 bytes --]

On Thu, 2010-12-23 at 09:21 -0800, Randy Dunlap wrote: 
> On Thu, 23 Dec 2010 17:21:43 +0100 Dario Faggioli wrote:
> 
> > Trying to read CLOCK_THREAD_CPUTIME_ID of a thread from outside
> > the process that spawned it with this code:
> > 
> >         if (clock_getcpuclockid(tid, &clockid) != 0) {
> >                 perror("clock_getcpuclockid");
> >                 exit(EXIT_FAILURE);
> >         }
> > 
> > results in this:
> >   ### Testing tid 24207: CPU-time clock for PID 24207 is 1.132371729 seconds
> >   ### Testing tid 24209: clock_getcpuclockid: Success
> > 
> > OTH, if full-fledged processes are involved, the behaviour is this:
> >   ### Testing tid 24218: CPU-time clock for PID 24218 is 0.001059305 seconds
> >   ### Testing tid 24220: CPU-time clock for PID 24220 is 1.044057391 seconds
> > 
> > Test programs available here: http://gitorious.org/clockid.
> 
> 
> "DNS service for this domain has expired with DNS Made Easy"  :(
> 
Yep, I saw that... It seems gitorious is having some sort of big big
problem! :-O

I've to run now, so, here's a really quick-&-dirty tar of the repo...

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
----------------------------------------------------------------------
Dario Faggioli, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa  (Italy)

http://retis.sssup.it/people/faggioli -- dario.faggioli@jabber.org

[-- Attachment #1.2: clockid.tar.bz2 --]
[-- Type: application/x-bzip-compressed-tar, Size: 23242 bytes --]

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[PATCH resend] Reading POSIX CPU timer from outside the process.

[Dario Faggioli]

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

Trying to call clock_getcpuclockid (and then clock_gettime) on a thread
from outside the process that spawned it results in this:
  ### Testing tid 24207: CPU-time clock for PID 24207 is 1.132371729 seconds
  ### Testing tid 24209: clock_getcpuclockid: Success

OTOH, if full-fledged processes are involved, the behaviour is this:
  ### Testing tid 24218: CPU-time clock for PID 24218 is 0.001059305 seconds
  ### Testing tid 24220: CPU-time clock for PID 24220 is 1.044057391 seconds

Test programs available here: http://gitorious.org/clockid

All that because clock_getcpuclockid forbids accessing thread
specific CPU-time clocks from outside the thread group, but this is not
requested (e.g., by POSIX), or at least no indication that it should fail
can be found in `man clock_getcpuclockid' and alike.
Moreover, having such capability could be useful, if you want to monitor
the execution of a bunch of thread from some kind of "manager" which might
not be part of the same process. A typical example is the JACK graph-manager.

Therefore, this commit makes clock_getcpuclockid behave as follows:
 - if it's called on a tid which is also a PID (i.e., the thread is
   a thread group leader), it returns the CLOCK_PROCESS_CPUTIME_ID of
   the process;
 - if it's called on a tid of a non-group leader, it returns the
   CLOCK_THREAD_CPUTIME_ID of such specific thread.

This enables the following behaviour, for the threaded and process-based
scenarios, respectively:

  ### Testing tid 24704: CPU-time clock for PID 24704 is 1.049570008 seconds
  ### Testing tid 24706: CPU-time clock for PID 24706 is 1.028650801 seconds

  ### Testing tid 24715: CPU-time clock for PID 24715 is 0.000957685 seconds
  ### Testing tid 24717: CPU-time clock for PID 24717 is 1.045351509 seconds

Signed-off-by: Dario Faggioli <raistlin@linux.it>
---
 kernel/posix-cpu-timers.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
index 05bb717..ddbbabc 100644
--- a/kernel/posix-cpu-timers.c
+++ b/kernel/posix-cpu-timers.c
@@ -39,10 +39,9 @@ static int check_clock(const clockid_t which_clock)
 
 	rcu_read_lock();
 	p = find_task_by_vpid(pid);
-	if (!p || !(CPUCLOCK_PERTHREAD(which_clock) ?
-		   same_thread_group(p, current) : has_group_leader_pid(p))) {
+	if (!p || (CPUCLOCK_PERTHREAD(which_clock) &&
+		   same_thread_group(p, current) && !has_group_leader_pid(p)))
 		error = -EINVAL;
-	}
 	rcu_read_unlock();
 
 	return error;
@@ -349,11 +348,9 @@ int posix_cpu_clock_get(const clockid_t which_clock, struct timespec *tp)
 		rcu_read_lock();
 		p = find_task_by_vpid(pid);
 		if (p) {
-			if (CPUCLOCK_PERTHREAD(which_clock)) {
-				if (same_thread_group(p, current)) {
-					error = cpu_clock_sample(which_clock,
-								 p, &rtn);
-				}
+			if (CPUCLOCK_PERTHREAD(which_clock) ||
+			    !thread_group_leader(p)) {
+				error = cpu_clock_sample(which_clock, p, &rtn);
 			} else {
 				read_lock(&tasklist_lock);
 				if (thread_group_leader(p) && p->sighand) {
-- 
1.7.2.3


-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
----------------------------------------------------------------------
Dario Faggioli, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa  (Italy)

http://retis.sssup.it/people/faggioli -- dario.faggioli@jabber.org

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH resend] Reading POSIX CPU timer from outside the process.

[Oleg Nesterov]

This patch doesn't look right,

On 12/28, Dario Faggioli wrote:
>
> Trying to call clock_getcpuclockid (and then clock_gettime) on a thread
> from outside the process that spawned it results in this:
>   ### Testing tid 24207: CPU-time clock for PID 24207 is 1.132371729 seconds
>   ### Testing tid 24209: clock_getcpuclockid: Success
>
> OTOH, if full-fledged processes are involved, the behaviour is this:
>   ### Testing tid 24218: CPU-time clock for PID 24218 is 0.001059305 seconds
>   ### Testing tid 24220: CPU-time clock for PID 24220 is 1.044057391 seconds
>
> Test programs available here: http://gitorious.org/clockid
>
> All that because clock_getcpuclockid forbids accessing thread
> specific CPU-time clocks from outside the thread group,

First of all, linux has no clock_getcpuclockid() system call, so
the changelog looks confusing.

glibc implements clock_getcpuclockid() via clock_getres(), that
is why the change in check_clock() can help.

> Therefore, this commit makes clock_getcpuclockid behave as follows:
>  - if it's called on a tid which is also a PID (i.e., the thread is
>    a thread group leader), it returns the CLOCK_PROCESS_CPUTIME_ID of
>    the process;
>  - if it's called on a tid of a non-group leader, it returns the
>    CLOCK_THREAD_CPUTIME_ID of such specific thread.

And both changes look wrong to me.

> --- a/kernel/posix-cpu-timers.c
> +++ b/kernel/posix-cpu-timers.c
> @@ -39,10 +39,9 @@ static int check_clock(const clockid_t which_clock)
>
>  	rcu_read_lock();
>  	p = find_task_by_vpid(pid);
> -	if (!p || !(CPUCLOCK_PERTHREAD(which_clock) ?
> -		   same_thread_group(p, current) : has_group_leader_pid(p))) {
> +	if (!p || (CPUCLOCK_PERTHREAD(which_clock) &&
> +		   same_thread_group(p, current) && !has_group_leader_pid(p)))
>  		error = -EINVAL;
> -	}
>  	rcu_read_unlock();

How so? For example, with this change
clock_getres(MAKE_THREAD_CPUCLOCK(pid_of_sub_thread)) won't work, no?

> @@ -349,11 +348,9 @@ int posix_cpu_clock_get(const clockid_t which_clock, struct timespec *tp)
>  		rcu_read_lock();
>  		p = find_task_by_vpid(pid);
>  		if (p) {
> -			if (CPUCLOCK_PERTHREAD(which_clock)) {
> -				if (same_thread_group(p, current)) {
> -					error = cpu_clock_sample(which_clock,
> -								 p, &rtn);
> -				}
> +			if (CPUCLOCK_PERTHREAD(which_clock) ||
> +			    !thread_group_leader(p)) {
> +				error = cpu_clock_sample(which_clock, p, &rtn);

IOW, we ignore CPUCLOCK_PERTHREAD() if !thread_group_leader(),
this can't be right.

I think, if we want to remove this limitation, we need something
like the patch below. If it doesn't help, we should fix glibc.

Oleg.

--- x/kernel/posix-cpu-timers.c
+++ x/kernel/posix-cpu-timers.c
@@ -39,10 +39,8 @@ static int check_clock(const clockid_t w
 
 	rcu_read_lock();
 	p = find_task_by_vpid(pid);
-	if (!p || !(CPUCLOCK_PERTHREAD(which_clock) ?
-		   same_thread_group(p, current) : has_group_leader_pid(p))) {
+	if (!p || !(CPUCLOCK_PERTHREAD(which_clock) || has_group_leader_pid(p)))
 		error = -EINVAL;
-	}
 	rcu_read_unlock();
 
 	return error;
@@ -350,10 +348,7 @@ int posix_cpu_clock_get(const clockid_t 
 		p = find_task_by_vpid(pid);
 		if (p) {
 			if (CPUCLOCK_PERTHREAD(which_clock)) {
-				if (same_thread_group(p, current)) {
-					error = cpu_clock_sample(which_clock,
-								 p, &rtn);
-				}
+				error = cpu_clock_sample(which_clock, p, &rtn);
 			} else {
 				read_lock(&tasklist_lock);
 				if (thread_group_leader(p) && p->sighand) {

Re: [PATCH resend] Reading POSIX CPU timer from outside the process.

[Dario Faggioli]

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

On Tue, 2010-12-28 at 17:38 +0100, Oleg Nesterov wrote: 
> This patch doesn't look right,
> 
Sorry then... :-(

> > All that because clock_getcpuclockid forbids accessing thread
> > specific CPU-time clocks from outside the thread group,
> 
> First of all, linux has no clock_getcpuclockid() system call, so
> the changelog looks confusing.
> 
Sure, you're right, this could have been more clear.

> >  	rcu_read_lock();
> >  	p = find_task_by_vpid(pid);
> > -	if (!p || !(CPUCLOCK_PERTHREAD(which_clock) ?
> > -		   same_thread_group(p, current) : has_group_leader_pid(p))) {
> > +	if (!p || (CPUCLOCK_PERTHREAD(which_clock) &&
> > +		   same_thread_group(p, current) && !has_group_leader_pid(p)))
> >  		error = -EINVAL;
> > -	}
> >  	rcu_read_unlock();
> 
> How so? For example, with this change
> clock_getres(MAKE_THREAD_CPUCLOCK(pid_of_sub_thread)) won't work, no?
> 
I tested all the clock_getres() calls that came to my mind (at least the
one that are possible from an userspace program), and they always worked
because of this (still in check_clock):

        const pid_t pid = CPUCLOCK_PID(which_clock);

        if (pid == 0)
                return 0;

Which triggers all the times, except when you actually try to get a CPU
clockid from outside the process, but that's not possible with getres.

Anyway, looking at the code again I agree, it may work, but it's not
something I really like! :-|

The whole point was about, given the current implementation of
clock_getcpuclockid done by glibc, can we remove that "failed with
success" (showed in the changelog) thing and come up with some
meaningful clockid for that situation? It's more than possible for the
answer to be no!!! :-P

> I think, if we want to remove this limitation, we need something
> like the patch below. If it doesn't help, we should fix glibc.
> 
> --- x/kernel/posix-cpu-timers.c
> +++ x/kernel/posix-cpu-timers.c
> @@ -39,10 +39,8 @@ static int check_clock(const clockid_t w
>  
>  	rcu_read_lock();
>  	p = find_task_by_vpid(pid);
> -	if (!p || !(CPUCLOCK_PERTHREAD(which_clock) ?
> -		   same_thread_group(p, current) : has_group_leader_pid(p))) {
> +	if (!p || !(CPUCLOCK_PERTHREAD(which_clock) || has_group_leader_pid(p)))
>  		error = -EINVAL;
> -	}
>  	rcu_read_unlock();
>
Which won't work because CPUCLOCK_PERTHREAD(which_clock) is always false
in this case. 

> 	return error;
> @@ -350,10 +348,7 @@ int posix_cpu_clock_get(const clockid_t 
>  		p = find_task_by_vpid(pid);
>  		if (p) {
>  			if (CPUCLOCK_PERTHREAD(which_clock)) {
> -				if (same_thread_group(p, current)) {
> -					error = cpu_clock_sample(which_clock,
> -								 p, &rtn);
> -				}
> +				error = cpu_clock_sample(which_clock, p, &rtn);
Same as above... To the point that I'm now wondering if we ever take
this branch here...

BTW, again, I see your point, the fix might need to happen at glibc
level. I'll check that and come back if I find something interesting.

Thanks anyway,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
----------------------------------------------------------------------
Dario Faggioli, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa  (Italy)

http://retis.sssup.it/people/faggioli -- dario.faggioli@jabber.org

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH resend] Reading POSIX CPU timer from outside the process.

[Oleg Nesterov]

On 12/28, Dario Faggioli wrote:
>
> On Tue, 2010-12-28 at 17:38 +0100, Oleg Nesterov wrote:
>
> > >  	rcu_read_lock();
> > >  	p = find_task_by_vpid(pid);
> > > -	if (!p || !(CPUCLOCK_PERTHREAD(which_clock) ?
> > > -		   same_thread_group(p, current) : has_group_leader_pid(p))) {
> > > +	if (!p || (CPUCLOCK_PERTHREAD(which_clock) &&
> > > +		   same_thread_group(p, current) && !has_group_leader_pid(p)))
> > >  		error = -EINVAL;
> > > -	}
> > >  	rcu_read_unlock();
> >
> > How so? For example, with this change
> > clock_getres(MAKE_THREAD_CPUCLOCK(pid_of_sub_thread)) won't work, no?
> >
> I tested all the clock_getres() calls that came to my mind (at least the
> one that are possible from an userspace program), and they always worked
> because of this (still in check_clock):
>
>         const pid_t pid = CPUCLOCK_PID(which_clock);
>
>         if (pid == 0)
>                 return 0;
>
> Which triggers all the times,

No, please note pid_of_sub_thread above.

> The whole point was about, given the current implementation of
> clock_getcpuclockid done by glibc, can we remove that "failed with
> success" (showed in the changelog) thing and come up with some
> meaningful clockid for that situation? It's more than possible for the
> answer to be no!!! :-P

I think we should change glibc if clock_getcpuclockid() doesn't work,
please see below.

> > I think, if we want to remove this limitation, we need something
> > like the patch below. If it doesn't help, we should fix glibc.
> >
> > --- x/kernel/posix-cpu-timers.c
> > +++ x/kernel/posix-cpu-timers.c
> > @@ -39,10 +39,8 @@ static int check_clock(const clockid_t w
> >
> >  	rcu_read_lock();
> >  	p = find_task_by_vpid(pid);
> > -	if (!p || !(CPUCLOCK_PERTHREAD(which_clock) ?
> > -		   same_thread_group(p, current) : has_group_leader_pid(p))) {
> > +	if (!p || !(CPUCLOCK_PERTHREAD(which_clock) || has_group_leader_pid(p)))
> >  		error = -EINVAL;
> > -	}
> >  	rcu_read_unlock();
> >
> Which won't work because CPUCLOCK_PERTHREAD(which_clock) is always false
> in this case.

I guess, this is because glibc passes MAKE_PROCESS_CLOCK() id, right?
But we shouldn't add the hacks to the kernel to hide the limitations
in glibc.

> BTW, again, I see your point, the fix might need to happen at glibc
> level. I'll check that and come back if I find something interesting.

Yes, please.


BTW. What is the test-case? I am looking at http://gitorious.org/clockid,
I guess it is clockid.c...

You do not need clock_getcpuclockid() at all. In fact I do not really
understand what this helper should actually do, probably it is only
needed to validate the pid. You can simply use MAKE_THREAD_CPUCLOCK()
to sample a single thread via clock_gettime().

IOW. Unless I missed something, with this patch, the only problem
is that getcpuclockid() always assumes MAKE_PROCESS_CPUCLOCK(),
I do not think this is the kernel problem.

Oleg.

Re: [PATCH resend] Reading POSIX CPU timer from outside the process.

[Dario Faggioli]

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

On Wed, 2010-12-29 at 14:21 +0100, Oleg Nesterov wrote: 
> > > How so? For example, with this change
> > > clock_getres(MAKE_THREAD_CPUCLOCK(pid_of_sub_thread)) won't work, no?
> > >
> > I tested all the clock_getres() calls that came to my mind (at least the
> > one that are possible from an userspace program), and they always worked
> > because of this (still in check_clock):
> >
> >         const pid_t pid = CPUCLOCK_PID(which_clock);
> >
> >         if (pid == 0)
> >                 return 0;
> >
> > Which triggers all the times,
> 
> No, please note pid_of_sub_thread above.
> 
Sure, I saw that... I was referring to all the clock_getres() and
clock_{get,set}time() I was able to call without using directly
MAKE_THREAD_CPUCLOCK.

> > Which won't work because CPUCLOCK_PERTHREAD(which_clock) is always false
> > in this case.
> 
> I guess, this is because glibc passes MAKE_PROCESS_CLOCK() id, right?
> But we shouldn't add the hacks to the kernel to hide the limitations
> in glibc.
> 
Exactly. Actually, I didn't noticed this too when I first started with
this patch, and the fact that my first version worked -- which was by
chance, I can say it now :-P -- made me think it was worthwhile to bend
the semantic a bit, to enable this new capability... But making it work
is hackish, I see it now. Sorry. :-)

> BTW. What is the test-case? I am looking at http://gitorious.org/clockid,
> I guess it is clockid.c...
> 
Yep. A very trivial one, I agree, but it's just to show the issue.

> You do not need clock_getcpuclockid() at all. In fact I do not really
> understand what this helper should actually do, probably it is only
> needed to validate the pid. You can simply use MAKE_THREAD_CPUCLOCK()
> to sample a single thread via clock_gettime().
> 
Fine, but, is that macro available for an application developer? Because
I can find it in kernel and glibc sources, but not in my /usr/include/*,
which is the motivation behind this attempt... But it might be my
fault! :-P

> IOW. Unless I missed something, with this patch, the only problem
> is that getcpuclockid() always assumes MAKE_PROCESS_CPUCLOCK(),
> I do not think this is the kernel problem.
> 
Agreed, sorry for wasting (hopefully not too much) people's time. :-(

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
----------------------------------------------------------------------
Dario Faggioli, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa  (Italy)

http://retis.sssup.it/people/faggioli -- dario.faggioli@jabber.org

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH resend] Reading POSIX CPU timer from outside the process.

[Oleg Nesterov]

On 12/29, Dario Faggioli wrote:
>
> On Wed, 2010-12-29 at 14:21 +0100, Oleg Nesterov wrote:
>
> > You do not need clock_getcpuclockid() at all. In fact I do not really
> > understand what this helper should actually do, probably it is only
> > needed to validate the pid. You can simply use MAKE_THREAD_CPUCLOCK()
> > to sample a single thread via clock_gettime().
> >
> Fine, but, is that macro available for an application developer? Because
> I can find it in kernel and glibc sources, but not in my /usr/include/*,
> which is the motivation behind this attempt... But it might be my
> fault! :-P

Yes, I do not see MAKE_*_CPUCLOCK() in /usr/include.

> > IOW. Unless I missed something, with this patch, the only problem
> > is that getcpuclockid() always assumes MAKE_PROCESS_CPUCLOCK(),
> > I do not think this is the kernel problem.
> >
> Agreed, sorry for wasting (hopefully not too much) people's time. :-(

No, I think you have a point. I'd suggest you to re-send the
patch which removes this limitation from kernel side.

My only objection was, we shouldn't add the hacks to overcome
the limitations in glibc. Say, posix_cpu_clock_get() should only
check CPUCLOCK_PERTHREAD(), it should not treat !group_leader
specially just because getcpuclockid() can construct the proper
clock id. This should be solved in userland.

Oleg.

Re: [PATCH resend] Reading POSIX CPU timer from outside the process.

[torbenh]

On Wed, Dec 29, 2010 at 02:21:30PM +0100, Oleg Nesterov wrote:
> On 12/28, Dario Faggioli wrote:
> >
> > On Tue, 2010-12-28 at 17:38 +0100, Oleg Nesterov wrote:
> >
> > > >  	rcu_read_lock();
> > > >  	p = find_task_by_vpid(pid);
> > > > -	if (!p || !(CPUCLOCK_PERTHREAD(which_clock) ?
> > > > -		   same_thread_group(p, current) : has_group_leader_pid(p))) {
> > > > +	if (!p || (CPUCLOCK_PERTHREAD(which_clock) &&
> > > > +		   same_thread_group(p, current) && !has_group_leader_pid(p)))
> > > >  		error = -EINVAL;
> > > > -	}
> > > >  	rcu_read_unlock();
> > >
> > > How so? For example, with this change
> > > clock_getres(MAKE_THREAD_CPUCLOCK(pid_of_sub_thread)) won't work, no?
> > >
> > I tested all the clock_getres() calls that came to my mind (at least the
> > one that are possible from an userspace program), and they always worked
> > because of this (still in check_clock):
> >
> >         const pid_t pid = CPUCLOCK_PID(which_clock);
> >
> >         if (pid == 0)
> >                 return 0;
> >
> > Which triggers all the times,
> 
> No, please note pid_of_sub_thread above.
> 
> > The whole point was about, given the current implementation of
> > clock_getcpuclockid done by glibc, can we remove that "failed with
> > success" (showed in the changelog) thing and come up with some
> > meaningful clockid for that situation? It's more than possible for the
> > answer to be no!!! :-P
> 
> I think we should change glibc if clock_getcpuclockid() doesn't work,
> please see below.

http://www.kernel.org/doc/man-pages/online/pages/man3/pthread_getcpuclockid.3.html

this one works.
ok... it takes a pthread_t for identifying the thread.
but it works.

> 
> > > I think, if we want to remove this limitation, we need something
> > > like the patch below. If it doesn't help, we should fix glibc.
> > >
> > > --- x/kernel/posix-cpu-timers.c
> > > +++ x/kernel/posix-cpu-timers.c
> > > @@ -39,10 +39,8 @@ static int check_clock(const clockid_t w
> > >
> > >  	rcu_read_lock();
> > >  	p = find_task_by_vpid(pid);
> > > -	if (!p || !(CPUCLOCK_PERTHREAD(which_clock) ?
> > > -		   same_thread_group(p, current) : has_group_leader_pid(p))) {
> > > +	if (!p || !(CPUCLOCK_PERTHREAD(which_clock) || has_group_leader_pid(p)))
> > >  		error = -EINVAL;
> > > -	}
> > >  	rcu_read_unlock();
> > >
> > Which won't work because CPUCLOCK_PERTHREAD(which_clock) is always false
> > in this case.
> 
> I guess, this is because glibc passes MAKE_PROCESS_CLOCK() id, right?
> But we shouldn't add the hacks to the kernel to hide the limitations
> in glibc.
> 
> > BTW, again, I see your point, the fix might need to happen at glibc
> > level. I'll check that and come back if I find something interesting.
> 
> Yes, please.
> 
> 
> BTW. What is the test-case? I am looking at http://gitorious.org/clockid,
> I guess it is clockid.c...
> 
> You do not need clock_getcpuclockid() at all. In fact I do not really
> understand what this helper should actually do, probably it is only
> needed to validate the pid. You can simply use MAKE_THREAD_CPUCLOCK()
> to sample a single thread via clock_gettime().
> 
> IOW. Unless I missed something, with this patch, the only problem
> is that getcpuclockid() always assumes MAKE_PROCESS_CPUCLOCK(),
> I do not think this is the kernel problem.
> 
> Oleg.
> 

-- 
torben Hohn

Re: [PATCH resend] Reading POSIX CPU timer from outside the process.

[Dario Faggioli]

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

On Thu, 2010-12-30 at 18:45 +0100, torbenh wrote: 
> > I think we should change glibc if clock_getcpuclockid() doesn't work,
> > please see below.
> 
> http://www.kernel.org/doc/man-pages/online/pages/man3/pthread_getcpuclockid.3.html
> 
> this one works.
> ok... it takes a pthread_t for identifying the thread.
> but it works.
> 
Well, I knew this, I knew it very well, as you can see from the code on
the git I pointed to (it's part of one of the examples!).

the thing is that I believe there are situations where it could be
useful to sample CLOCK_THREAD_CPUTIME_ID from a _different_ process, and
in that case, you can't access that thread's pthread-id, can you? :-O

The jack2 code I saw could be one of these "potential user", and AFAICT
your jack1 might be another one, no?
Yeah, I know, whether you can/want use this or not also depends on other
issues, but you are in a _different_ process --and thus you can't use
pthread_* calls-- aren't you?

I'm now looking at Oleg's solution and into glibc as well... I'll come
out with something ASAP.

Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
----------------------------------------------------------------------
Dario Faggioli, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa  (Italy)

http://retis.sssup.it/people/faggioli -- dario.faggioli@jabber.org

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH resend] Reading POSIX CPU timer from outside the process.

[torbenh]

On Tue, Jan 04, 2011 at 12:01:25PM +0100, Dario Faggioli wrote:
> On Thu, 2010-12-30 at 18:45 +0100, torbenh wrote: 
> > > I think we should change glibc if clock_getcpuclockid() doesn't work,
> > > please see below.
> > 
> > http://www.kernel.org/doc/man-pages/online/pages/man3/pthread_getcpuclockid.3.html
> > 
> > this one works.
> > ok... it takes a pthread_t for identifying the thread.
> > but it works.
> > 
> Well, I knew this, I knew it very well, as you can see from the code on
> the git I pointed to (it's part of one of the examples!).
> 
> the thing is that I believe there are situations where it could be
> useful to sample CLOCK_THREAD_CPUTIME_ID from a _different_ process, and
> in that case, you can't access that thread's pthread-id, can you? :-O
> 
> The jack2 code I saw could be one of these "potential user", and AFAICT
> your jack1 might be another one, no?
> Yeah, I know, whether you can/want use this or not also depends on other
> issues, but you are in a _different_ process --and thus you can't use
> pthread_* calls-- aren't you?

i would need to tell the tid to jackd anyways.
i could as well just communicate the clock_id. 

getting the clock_id into the other process is not a problem.
the problem is that i am not allowed to read out the clock.


> 
> I'm now looking at Oleg's solution and into glibc as well... I'll come
> out with something ASAP.
> 
> Regards,
> Dario
> 
> -- 
> <<This happens because I choose it to happen!>> (Raistlin Majere)
> ----------------------------------------------------------------------
> Dario Faggioli, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa  (Italy)
> 
> http://retis.sssup.it/people/faggioli -- dario.faggioli@jabber.org



-- 
torben Hohn

Re: [PATCH] Read THREAD_CPUTIME clock from other processes.

[Roland McGrath]

clock_getcpuclockid is the POSIX interface for using a process-wide CPU
clock.  pthread_getcpuclockid is the POSIX interface for using a
thread-specific CPU clock.  There is no POSIX interface for using the
thread-specific clock of a thread in a different process because POSIX does
not have the notion of global identification of threads at all.  The very
idea that you could know anything about an individual thread in a different
process is a Linuxism.  If you want to do something like that, then there
is no reason to use the POSIX standard interfaces rather than just using
the Linux-specific clockid_t generation macros in the first place.

When the CPU clock interfaces were introduced to the kernel, it was
considered a potential security issue (information leak) to be able to
access the thread clocks of another process, because there had never been a
way for one process to access such information from another process before.
We took the conservative route of permitting it only within the same
process.  

This can certainly be enhanced, but it opens some cans of worms about the
security question.  It is probably still considered an unsafe information
leak to let every process examine every other process's thread clocks.
I'll leave that judgement to security folks.  The intermediate route of
conversatism is to allow it only for processes owned by the same user,
which has some complexities with races between UID changes and clock/timer
calls.

As well as the information leak, it is most certainly a DoS attack vector
to allow one process to set CPU timers an another process or its threads.
Setting timers causes the timed thread itself to do work proportional to
the number of timers set.


Thanks,
Roland

Re: [PATCH] Read THREAD_CPUTIME clock from other processes.

[Oleg Nesterov]

On 01/07, Roland McGrath wrote:
>
> This can certainly be enhanced, but it opens some cans of worms about the
> security question.  It is probably still considered an unsafe information
> leak to let every process examine every other process's thread clocks.

Yes, I was worried about possible security issues too. But, it seems,
/proc/pid/task/tid/stat (do_task_stat) shows ->utime/stime anyway.
And /proc/schedstat shows sum_exec_runtime.

> I'll leave that judgement to security folks.

Agreed.

> As well as the information leak, it is most certainly a DoS attack vector
> to allow one process to set CPU timers an another process or its threads.

No, the suggested change doesn't go that far, afaics. It only modifies
check_clock: this affects clock_getres and clock_set (which does nothing),
and posix_cpu_clock_get: affects clock_gettime().

Oleg.

Re: [PATCH] Read THREAD_CPUTIME clock from other processes.

[Roland McGrath]

> Yes, I was worried about possible security issues too. But, it seems,
> /proc/pid/task/tid/stat (do_task_stat) shows ->utime/stime anyway.

This is constrained by ptrace_may_access(task, PTRACE_MODE_READ).

> And /proc/schedstat shows sum_exec_runtime.

This probably should be constrained in exactly the same ways as .../stat is.

I think the constraints on /proc/.../stat reflect the most considered
judgement of the security folks.  I suspect that the lack of constraint on
/proc/.../schedstat reflects the scheduler folks just having failed to
consider the same issues.  IMHO all access to the equivalent kinds of
information should have the same security constraints.

> No, the suggested change doesn't go that far, afaics. It only modifies
> check_clock: this affects clock_getres and clock_set (which does nothing),
> and posix_cpu_clock_get: affects clock_gettime().

That's good, but it should be mentioned so that next year someone doesn't
come back with a "there's no reason you shouldn't be able to" and go
unchallenged.


Thanks,
Roland

Re: [PATCH] Read THREAD_CPUTIME clock from other processes.

[Oleg Nesterov]

On 01/07, Roland McGrath wrote:
>
> > Yes, I was worried about possible security issues too. But, it seems,
> > /proc/pid/task/tid/stat (do_task_stat) shows ->utime/stime anyway.
>
> This is constrained by ptrace_may_access(task, PTRACE_MODE_READ).

Well, ptrace_may_access() protects only get_wchan/eip/esp, but
not utime/stime.

> That's good, but it should be mentioned so that next year someone doesn't
> come back with a "there's no reason you shouldn't be able to" and go
> unchallenged.

Yes, agreed. The changelog should be improved.

Oleg.

Re: [PATCH] Read THREAD_CPUTIME clock from other processes.

[Roland McGrath]

> Well, ptrace_may_access() protects only get_wchan/eip/esp, but
> not utime/stime.

Oh, sorry.  I misread the code.  As I said, I leave it to the security
folks to decide what the appropriate constraints are.  All the mechanisms
for accessing the same information should just be consistent.  So perhaps
this is indeed fine.


Thanks,
Roland

Re: [PATCH] Read THREAD_CPUTIME clock from other processes.

[Peter Zijlstra]

On Fri, 2011-01-07 at 11:50 -0800, Roland McGrath wrote:
> I think the constraints on /proc/.../stat reflect the most considered
> judgement of the security folks.  I suspect that the lack of constraint on
> /proc/.../schedstat reflects the scheduler folks just having failed to
> consider the same issues.  IMHO all access to the equivalent kinds of
> information should have the same security constraints. 

FWIW I'm fine with adding a PTRACE_MAY_READ constraint on schedstat.

Re: [PATCH] Read THREAD_CPUTIME clock from other processes.

[Dario Faggioli]

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

On Fri, 2011-01-07 at 11:28 -0800, Roland McGrath wrote: 
> clock_getcpuclockid is the POSIX interface for using a process-wide CPU
> clock.  pthread_getcpuclockid is the POSIX interface for using a
> thread-specific CPU clock.  There is no POSIX interface for using the
> thread-specific clock of a thread in a different process because POSIX does
> not have the notion of global identification of threads at all.
>
Sure, even just the fact that the tid is involved makes it unequivocally
a Linux extension to such interface.

> The very
> idea that you could know anything about an individual thread in a different
> process is a Linuxism.  If you want to do something like that, then there
> is no reason to use the POSIX standard interfaces rather than just using
> the Linux-specific clockid_t generation macros in the first place.
> 
And I'm perfectly fine with this, if there's a consensus around it! :-)

The whole point is that this is not possible right now, since the
clockid_t generators are not accessible from userspace... Should I go
for this?

> When the CPU clock interfaces were introduced to the kernel, it was
> considered a potential security issue (information leak) to be able to
> access the thread clocks of another process, because there had never been a
> way for one process to access such information from another process before.
> We took the conservative route of permitting it only within the same
> process.  
> 
That crossed my mind as well (I think I mentioned at least in one of the
e-mail if not in the changelog). Then I thought that if it's possible to
access an arbitrary process' CPU timer, why it shouldn't be possible to
access the one of an arbitrary thread... But, as you, I'm not a
"security guy", and I would be the first one to suggest to drop this if
it is considered a security risk! :-)

> As well as the information leak, it is most certainly a DoS attack vector
> to allow one process to set CPU timers an another process or its threads.
> Setting timers causes the timed thread itself to do work proportional to
> the number of timers set.
> 
Yep, but as said, no room for timer setting, neither with or without the
patch, due to the nature of the clock itlsef.

Thanks,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
----------------------------------------------------------------------
Dario Faggioli, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa  (Italy)

http://retis.sssup.it/people/faggioli -- dario.faggioli@jabber.org

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]