* [PATCH] posix_cpu_timers: fix timer never expires when executes clock_nanosleep
@ 2013-08-01 10:10 Dong Zhu
2013-08-01 11:30 ` Stanislaw Gruszka
0 siblings, 1 reply; 3+ messages in thread
From: Dong Zhu @ 2013-08-01 10:10 UTC (permalink / raw)
To: John Stultz, Thomas Gleixner, Stanislaw Gruszka, Oleg Nesterov,
Ingo Molnar
Cc: linux-kernel
>From c7439b90b0794c016b29356f0e232f7413ef7b60 Mon Sep 17 00:00:00 2001
From: Dong Zhu <bluezhudong@gmail.com>
Date: Thu, 1 Aug 2013 11:39:04 +0800
When use the current process pid as the clockid, then executes
clock_nanosleep syscall the timer will never expire. Kernel should
prevent user doing like this and this patch is supposed to fix it.I
wrote a simple case to test it:
#include <time.h>
#include <errno.h>
#include <unistd.h>
#include <sys/types.h>
#define CPU_CLOCK_PROF 0
#define CPU_CLOCK_VIRT 1
#define CPU_CLOCK_SCHED 2
#define CPU_CLOCK_THREAD 4
#define PID_TO_CLOCKID(pid, clock) ((~(clockid_t) (pid) << 3) | (clockid_t) (clock))
int main(void)
{
int ret;
pid_t pid;
clockid_t clk;
struct timespec ts;
ts.tv_sec = 1;
ts.tv_nsec = 0;
pid = getpid();
clk = PID_TO_CLOCKID(pid, CPU_CLOCK_PROF);
if ((ret = clock_nanosleep(clk, 0, &ts, NULL)) != 0) {
perror("clock_nanosleep");
return ret;
}
return 0;
}
Signed-off-by: Dong Zhu <bluezhudong@gmail.com>
---
kernel/posix-cpu-timers.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
index c7f31aa..cc03290 100644
--- a/kernel/posix-cpu-timers.c
+++ b/kernel/posix-cpu-timers.c
@@ -1413,9 +1413,9 @@ static int posix_cpu_nsleep(const clockid_t which_clock, int flags,
/*
* Diagnose required errors first.
*/
- if (CPUCLOCK_PERTHREAD(which_clock) &&
- (CPUCLOCK_PID(which_clock) == 0 ||
- CPUCLOCK_PID(which_clock) == current->pid))
+ if (CPUCLOCK_PID(which_clock) == current->pid ||
+ (CPUCLOCK_PERTHREAD(which_clock) &&
+ CPUCLOCK_PID(which_clock) == 0))
return -EINVAL;
error = do_cpu_nanosleep(which_clock, flags, rqtp, &it);
--
1.7.11.7
--
Best Regards,
Dong Zhu
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] posix_cpu_timers: fix timer never expires when executes clock_nanosleep
2013-08-01 10:10 [PATCH] posix_cpu_timers: fix timer never expires when executes clock_nanosleep Dong Zhu
@ 2013-08-01 11:30 ` Stanislaw Gruszka
2013-08-01 13:11 ` Dong Zhu
0 siblings, 1 reply; 3+ messages in thread
From: Stanislaw Gruszka @ 2013-08-01 11:30 UTC (permalink / raw)
To: Dong Zhu
Cc: John Stultz, Thomas Gleixner, Oleg Nesterov, Ingo Molnar,
linux-kernel
Hi Dong Zhu
On Thu, Aug 01, 2013 at 06:10:19PM +0800, Dong Zhu wrote:
> diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
> index c7f31aa..cc03290 100644
> --- a/kernel/posix-cpu-timers.c
> +++ b/kernel/posix-cpu-timers.c
> @@ -1413,9 +1413,9 @@ static int posix_cpu_nsleep(const clockid_t which_clock, int flags,
> /*
> * Diagnose required errors first.
> */
> - if (CPUCLOCK_PERTHREAD(which_clock) &&
> - (CPUCLOCK_PID(which_clock) == 0 ||
> - CPUCLOCK_PID(which_clock) == current->pid))
> + if (CPUCLOCK_PID(which_clock) == current->pid ||
> + (CPUCLOCK_PERTHREAD(which_clock) &&
> + CPUCLOCK_PID(which_clock) == 0))
> return -EINVAL;
Nope, this is wrong. We have to allow own pid process clock, because it
can be used correctly on multi-threaded processes. Own tid thread clock
has no sense and we correctly return -EINVAL in such case.
We could possibly add check for own pid together with check if process
consist of one thread, but that is too complicated IMHO especially
taking into account that threads on the process can be destroyed and
created dynamically.
Stanislaw
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] posix_cpu_timers: fix timer never expires when executes clock_nanosleep
2013-08-01 11:30 ` Stanislaw Gruszka
@ 2013-08-01 13:11 ` Dong Zhu
0 siblings, 0 replies; 3+ messages in thread
From: Dong Zhu @ 2013-08-01 13:11 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: John Stultz, Thomas Gleixner, Oleg Nesterov, Ingo Molnar,
linux-kernel
Hi Stanislaw,
Thansk for your info.
On Thu, Aug 01, 2013 at 01:30:50PM +0200, Stanislaw Gruszka wrote:
> Hi Dong Zhu
>
> On Thu, Aug 01, 2013 at 06:10:19PM +0800, Dong Zhu wrote:
> > diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
> > index c7f31aa..cc03290 100644
> > --- a/kernel/posix-cpu-timers.c
> > +++ b/kernel/posix-cpu-timers.c
> > @@ -1413,9 +1413,9 @@ static int posix_cpu_nsleep(const clockid_t which_clock, int flags,
> > /*
> > * Diagnose required errors first.
> > */
> > - if (CPUCLOCK_PERTHREAD(which_clock) &&
> > - (CPUCLOCK_PID(which_clock) == 0 ||
> > - CPUCLOCK_PID(which_clock) == current->pid))
> > + if (CPUCLOCK_PID(which_clock) == current->pid ||
> > + (CPUCLOCK_PERTHREAD(which_clock) &&
> > + CPUCLOCK_PID(which_clock) == 0))
> > return -EINVAL;
>
> Nope, this is wrong. We have to allow own pid process clock, because it
> can be used correctly on multi-threaded processes. Own tid thread clock
Yes, you are right, I really neglected this point.
> has no sense and we correctly return -EINVAL in such case.
>
> We could possibly add check for own pid together with check if process
> consist of one thread, but that is too complicated IMHO especially
> taking into account that threads on the process can be destroyed and
> created dynamically.
>
Agree, really so complicated.
--
Best Regards,
Dong Zhu
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-08-01 13:12 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-01 10:10 [PATCH] posix_cpu_timers: fix timer never expires when executes clock_nanosleep Dong Zhu
2013-08-01 11:30 ` Stanislaw Gruszka
2013-08-01 13:11 ` Dong Zhu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).