lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 14 Apr 2022 13:54:12 +0200 From: Oleg Nesterov <oleg@...hat.com> To: Peter Zijlstra <peterz@...radead.org> Cc: rjw@...ysocki.net, mingo@...nel.org, vincent.guittot@...aro.org, dietmar.eggemann@....com, rostedt@...dmis.org, mgorman@...e.de, ebiederm@...ssion.com, bigeasy@...utronix.de, Will Deacon <will@...nel.org>, linux-kernel@...r.kernel.org, tj@...nel.org, linux-pm@...r.kernel.org Subject: Re: [PATCH 2/5] sched,ptrace: Fix ptrace_check_attach() vs PREEMPT_RT On 04/13, Peter Zijlstra wrote: > > On Wed, Apr 13, 2022 at 09:20:53PM +0200, Peter Zijlstra wrote: > > On Wed, Apr 13, 2022 at 08:59:10PM +0200, Oleg Nesterov wrote: > > > > > > > + // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! > > > + // wrong, needs siglock > > > + current->jobctl &= ~JOBCTL_TRACED_XXX; > > > + wake_up_bit(¤t->jobctl, ~JOBCTL_TRACED_XXX_BIT); > > __wake_up_common_lock() > > spin_lock_irqsave() > > current_save_and_set_rtlock_wait_state(); OOPS, thanks. > Something that might work; since there's only the one ptracer, is to > instead do something like: > > current->jobctl &= ~JOBCTL_TRACED_XXX; // siglock > if (current->ptrace) > wake_up_process(current->parent); > preempt_enable_no_resched(); > schedule(); > > > vs > > for (;;) { > set_current_state(TASK_UNINTERRUPTIBLE); > if (!(p->jobctl & JOBCTL_TRACED_XXX)) > break; > schedule(); Yes, thanks... this is racy, see below, but probably fixeable. > ptrace_detach() needs some additional magic as well I think, but this > might just work. I don't think so, JOBCTL_TRACED_XXX must be always cleared in ptrace_stop() and ptrace_detach() implies ptrace_check_attach(). Lets forget about the proble above for the moment. There is another problem with my patch, if (!(child->ptrace && child->parent == current)) return ret; this check is racy without tasklist, we can race with another task attaching to our natural child (so that child->parent == current), ptrace_attach() sets task->ptrace = flags first and changes child->parent after that. In this case: if (ignore_state) return 0; this is just wrong, if (wait_on_bit(&task->jobctl, JOBCTL_TRACED_XXX_BIT, TASK_KILLABLE)) return -EINTR; this is fine, if (!wait_task_inactive(child, __TASK_TRACED)) this not right too. wait_task_inactive() can loop "forever" doing schedule_hrtimeout() if the actual debugger stops/resumes the tracee continuously. This is pure theoretical, but still. And this also means that the code above needs some changes too, we can rely on wake_up_process(current->parent). OK, let me think about it. Thanks! Oleg.
Powered by blists - more mailing lists