* [PATCH 1/3] posix_timer: remove wrong comment
@ 2010-05-17 13:41 Andrey Vagin
2010-05-17 13:41 ` [PATCH 2/3] posix_timer: fix error path in timer_create Andrey Vagin
0 siblings, 1 reply; 7+ messages in thread
From: Andrey Vagin @ 2010-05-17 13:41 UTC (permalink / raw)
To: Thomas Gleixner, Andrew Morton
Cc: linux-kernel, stable, Oleg Nesterov, Pavel Emelyanov,
Stanislaw Gruszka, Andrey Vagin
currently "The next step is hard to back out if there is an error." comment is not right,
release_posix_timer() does put_pid().
Signed-off-by: Andrey Vagin <avagin@openvz.org>
---
kernel/posix-timers.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
index 00d1fda..5555e7c 100644
--- a/kernel/posix-timers.c
+++ b/kernel/posix-timers.c
@@ -563,10 +563,6 @@ SYSCALL_DEFINE3(timer_create, const clockid_t, which_clock,
if (error)
goto out;
- /*
- * return the timer_id now. The next step is hard to
- * back out if there is an error.
- */
if (copy_to_user(created_timer_id,
&new_timer_id, sizeof (new_timer_id))) {
error = -EFAULT;
--
1.6.6
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/3] posix_timer: fix error path in timer_create
2010-05-17 13:41 [PATCH 1/3] posix_timer: remove wrong comment Andrey Vagin
@ 2010-05-17 13:41 ` Andrey Vagin
2010-05-17 13:41 ` [PATCH 3/3] posix_timer: move copy_to_user(created_timer_id) down " Andrey Vagin
2010-05-17 14:31 ` [PATCH 2/3] posix_timer: fix error path " Stanislaw Gruszka
0 siblings, 2 replies; 7+ messages in thread
From: Andrey Vagin @ 2010-05-17 13:41 UTC (permalink / raw)
To: Thomas Gleixner, Andrew Morton
Cc: linux-kernel, stable, Oleg Nesterov, Pavel Emelyanov,
Stanislaw Gruszka, Andrey Vagin
move CLOCK_DISPATCH(which_clock, timer_create, (new_timer)) after all
possible EFAULT errors.
*_timer_create may allocate/get resources.
(for example posix_cpu_timer_create does get_task_struct)
Signed-off-by: Andrey Vagin <avagin@openvz.org>
---
kernel/posix-timers.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
index 5555e7c..8393624 100644
--- a/kernel/posix-timers.c
+++ b/kernel/posix-timers.c
@@ -555,14 +555,6 @@ SYSCALL_DEFINE3(timer_create, const clockid_t, which_clock,
goto out;
}
- it_id_set = IT_ID_SET;
- new_timer->it_id = (timer_t) new_timer_id;
- new_timer->it_clock = which_clock;
- new_timer->it_overrun = -1;
- error = CLOCK_DISPATCH(which_clock, timer_create, (new_timer));
- if (error)
- goto out;
-
if (copy_to_user(created_timer_id,
&new_timer_id, sizeof (new_timer_id))) {
error = -EFAULT;
@@ -593,6 +585,14 @@ SYSCALL_DEFINE3(timer_create, const clockid_t, which_clock,
new_timer->sigq->info.si_tid = new_timer->it_id;
new_timer->sigq->info.si_code = SI_TIMER;
+ it_id_set = IT_ID_SET;
+ new_timer->it_id = (timer_t) new_timer_id;
+ new_timer->it_clock = which_clock;
+ new_timer->it_overrun = -1;
+ error = CLOCK_DISPATCH(which_clock, timer_create, (new_timer));
+ if (error)
+ goto out;
+
spin_lock_irq(¤t->sighand->siglock);
new_timer->it_signal = current->signal;
list_add(&new_timer->list, ¤t->signal->posix_timers);
--
1.6.6
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/3] posix_timer: move copy_to_user(created_timer_id) down in timer_create
2010-05-17 13:41 ` [PATCH 2/3] posix_timer: fix error path in timer_create Andrey Vagin
@ 2010-05-17 13:41 ` Andrey Vagin
2010-05-17 14:31 ` [PATCH 2/3] posix_timer: fix error path " Stanislaw Gruszka
1 sibling, 0 replies; 7+ messages in thread
From: Andrey Vagin @ 2010-05-17 13:41 UTC (permalink / raw)
To: Thomas Gleixner, Andrew Morton
Cc: linux-kernel, stable, Oleg Nesterov, Pavel Emelyanov,
Stanislaw Gruszka, Andrey Vagin
According to Oleg Nesterov:
"We can move copy_to_user(created_timer_id) down after "if (timer_event_spec)"
block too. (but before CLOCK_DISPATCH(), of course)."
Signed-off-by: Andrey Vagin <avagin@openvz.org>
---
kernel/posix-timers.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
index 8393624..6da1752 100644
--- a/kernel/posix-timers.c
+++ b/kernel/posix-timers.c
@@ -555,11 +555,6 @@ SYSCALL_DEFINE3(timer_create, const clockid_t, which_clock,
goto out;
}
- if (copy_to_user(created_timer_id,
- &new_timer_id, sizeof (new_timer_id))) {
- error = -EFAULT;
- goto out;
- }
if (timer_event_spec) {
if (copy_from_user(&event, timer_event_spec, sizeof (event))) {
error = -EFAULT;
@@ -585,6 +580,12 @@ SYSCALL_DEFINE3(timer_create, const clockid_t, which_clock,
new_timer->sigq->info.si_tid = new_timer->it_id;
new_timer->sigq->info.si_code = SI_TIMER;
+ if (copy_to_user(created_timer_id,
+ &new_timer_id, sizeof (new_timer_id))) {
+ error = -EFAULT;
+ goto out;
+ }
+
it_id_set = IT_ID_SET;
new_timer->it_id = (timer_t) new_timer_id;
new_timer->it_clock = which_clock;
--
1.6.6
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 2/3] posix_timer: fix error path in timer_create
2010-05-17 13:41 ` [PATCH 2/3] posix_timer: fix error path in timer_create Andrey Vagin
2010-05-17 13:41 ` [PATCH 3/3] posix_timer: move copy_to_user(created_timer_id) down " Andrey Vagin
@ 2010-05-17 14:31 ` Stanislaw Gruszka
2010-05-17 14:55 ` Andrew Vagin
1 sibling, 1 reply; 7+ messages in thread
From: Stanislaw Gruszka @ 2010-05-17 14:31 UTC (permalink / raw)
To: Andrey Vagin
Cc: Thomas Gleixner, Andrew Morton, linux-kernel, stable,
Oleg Nesterov, Pavel Emelyanov, Andrey Vagin
On Mon, 17 May 2010 17:41:43 +0400
Andrey Vagin <avagin@openvz.org> wrote:
> move CLOCK_DISPATCH(which_clock, timer_create, (new_timer)) after all
> possible EFAULT errors.
>
> *_timer_create may allocate/get resources.
> (for example posix_cpu_timer_create does get_task_struct)
>
> Signed-off-by: Andrey Vagin <avagin@openvz.org>
> ---
> kernel/posix-timers.c | 16 ++++++++--------
> 1 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
> index 5555e7c..8393624 100644
> --- a/kernel/posix-timers.c
> +++ b/kernel/posix-timers.c
> @@ -555,14 +555,6 @@ SYSCALL_DEFINE3(timer_create, const clockid_t, which_clock,
> goto out;
> }
>
> - it_id_set = IT_ID_SET;
> - new_timer->it_id = (timer_t) new_timer_id;
> - new_timer->it_clock = which_clock;
> - new_timer->it_overrun = -1;
> - error = CLOCK_DISPATCH(which_clock, timer_create, (new_timer));
> - if (error)
> - goto out;
> -
> if (copy_to_user(created_timer_id,
> &new_timer_id, sizeof (new_timer_id))) {
> error = -EFAULT;
> @@ -593,6 +585,14 @@ SYSCALL_DEFINE3(timer_create, const clockid_t, which_clock,
> new_timer->sigq->info.si_tid = new_timer->it_id;
> new_timer->sigq->info.si_code = SI_TIMER;
>
> + it_id_set = IT_ID_SET;
> + new_timer->it_id = (timer_t) new_timer_id;
This part should not be moved, this make possible leak of idr entry .
> + new_timer->it_clock = which_clock;
> + new_timer->it_overrun = -1;
I'm not so convenient of moving this as well.
> + error = CLOCK_DISPATCH(which_clock, timer_create, (new_timer));
> + if (error)
> + goto out;
> +
> spin_lock_irq(¤t->sighand->siglock);
> new_timer->it_signal = current->signal;
> list_add(&new_timer->list, ¤t->signal->posix_timers);
Thanks
Stanislaw
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 2/3] posix_timer: fix error path in timer_create
2010-05-17 14:31 ` [PATCH 2/3] posix_timer: fix error path " Stanislaw Gruszka
@ 2010-05-17 14:55 ` Andrew Vagin
0 siblings, 0 replies; 7+ messages in thread
From: Andrew Vagin @ 2010-05-17 14:55 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: Andrey Vagin, Thomas Gleixner, Andrew Morton, linux-kernel,
stable, Oleg Nesterov, Pavel Emelyanov
On 05/17/2010 06:31 PM, Stanislaw Gruszka wrote:
> On Mon, 17 May 2010 17:41:43 +0400
> Andrey Vagin<avagin@openvz.org> wrote:
>
>
>> @@ -593,6 +585,14 @@ SYSCALL_DEFINE3(timer_create, const clockid_t, which_clock,
>> new_timer->sigq->info.si_tid = new_timer->it_id;
>> new_timer->sigq->info.si_code = SI_TIMER;
>>
>> + it_id_set = IT_ID_SET;
>> + new_timer->it_id = (timer_t) new_timer_id;
>>
> This part should not be moved, this make possible leak of idr entry .
>
You are right. Pls, skip this patches too.
>
>> + new_timer->it_clock = which_clock;
>> + new_timer->it_overrun = -1;
>>
> I'm not so convenient of moving this as well.
>
>
>> + error = CLOCK_DISPATCH(which_clock, timer_create, (new_timer));
>> + if (error)
>> + goto out;
>> +
>> spin_lock_irq(¤t->sighand->siglock);
>> new_timer->it_signal = current->signal;
>> list_add(&new_timer->list,¤t->signal->posix_timers);
>>
> Thanks
> Stanislaw
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] posix_timer: remove wrong comment
@ 2010-05-17 17:18 Andrey Vagin
2010-05-18 6:49 ` Stanislaw Gruszka
0 siblings, 1 reply; 7+ messages in thread
From: Andrey Vagin @ 2010-05-17 17:18 UTC (permalink / raw)
To: Thomas Gleixner, Andrew Morton
Cc: linux-kernel, stable, Oleg Nesterov, Pavel Emelyanov,
Stanislaw Gruszka, Andrey Vagin
currently "The next step is hard to back out if there is an error."
comment is not right, release_posix_timer() does put_pid().
Signed-off-by: Andrey Vagin <avagin@openvz.org>
---
kernel/posix-timers.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
index 00d1fda..5555e7c 100644
--- a/kernel/posix-timers.c
+++ b/kernel/posix-timers.c
@@ -563,10 +563,6 @@ SYSCALL_DEFINE3(timer_create, const clockid_t, which_clock,
if (error)
goto out;
- /*
- * return the timer_id now. The next step is hard to
- * back out if there is an error.
- */
if (copy_to_user(created_timer_id,
&new_timer_id, sizeof (new_timer_id))) {
error = -EFAULT;
--
1.6.6
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-05-18 6:47 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-17 13:41 [PATCH 1/3] posix_timer: remove wrong comment Andrey Vagin
2010-05-17 13:41 ` [PATCH 2/3] posix_timer: fix error path in timer_create Andrey Vagin
2010-05-17 13:41 ` [PATCH 3/3] posix_timer: move copy_to_user(created_timer_id) down " Andrey Vagin
2010-05-17 14:31 ` [PATCH 2/3] posix_timer: fix error path " Stanislaw Gruszka
2010-05-17 14:55 ` Andrew Vagin
-- strict thread matches above, loose matches on Subject: below --
2010-05-17 17:18 [PATCH 1/3] posix_timer: remove wrong comment Andrey Vagin
2010-05-18 6:49 ` Stanislaw Gruszka
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).