Messages in this thread | | | Date | Thu, 21 Jul 2011 20:34:46 +0200 | From | Oleg Nesterov <> | Subject | [PATCH 0/1] (Was: connector: add an event for monitoring process tracers) |
| |
On 07/18, Oleg Nesterov wrote: > > proc_fork_connector() reads task->real_parent lockless. In theory > this is not safe with CLONE_PTHREAD or CLONE_PARENT. Yes, this is > only theoretical, but afaics we need something like > > --- x/drivers/connector/cn_proc.c > +++ x/drivers/connector/cn_proc.c > @@ -55,6 +55,7 @@ void proc_fork_connector(struct task_str > struct proc_event *ev; > __u8 buffer[CN_PROC_MSG_SIZE]; > struct timespec ts; > + struct task_struct *parent; > > if (atomic_read(&proc_event_num_listeners) < 1) > return; > @@ -65,8 +66,11 @@ void proc_fork_connector(struct task_str > ktime_get_ts(&ts); /* get high res monotonic timestamp */ > put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns); > ev->what = PROC_EVENT_FORK; > - ev->event_data.fork.parent_pid = task->real_parent->pid; > - ev->event_data.fork.parent_tgid = task->real_parent->tgid; > + rcu_read_lock(); > + parent = rcu_dereference(task->real_parent); > + ev->event_data.fork.parent_pid = parent->pid; > + ev->event_data.fork.parent_tgid = parent->tgid; > + rcu_read_unlock(); > ev->event_data.fork.child_pid = task->pid; > ev->event_data.fork.child_tgid = task->tgid; > > Otherwise ->real_parent can point to the freed/reused and may be > unmapped memory.
Looks like, nobody cares ;) I am sending the patch.
> But the actual question is, the usage of proc_exec_connector() > looks "obviously wrong", no? Don't we need > > --- x/fs/exec.c > +++ x/fs/exec.c > @@ -1380,15 +1380,16 @@ int search_binary_handler(struct linux_b > */ > bprm->recursion_depth = depth; > if (retval >= 0) { > - if (depth == 0) > + if (depth == 0) { > tracehook_report_exec(fmt, bprm, regs); > + proc_exec_connector(current); > + } > put_binfmt(fmt); > allow_write_access(bprm->file); > if (bprm->file) > fput(bprm->file); > bprm->file = NULL; > current->did_exec = 1; > - proc_exec_connector(current); > return retval; > } > read_lock(&binfmt_lock); > > > ? Or do we really want to call proc_exec_connector() twice or > more in "#!whatever" case?
I think this should be fixed too, I'll send the patch later.
Oleg.
| |