Message ID | 1551854787-12927-1-git-send-email-lirongqing@baidu.com |
---|---|
State | Accepted |
Delegated to: | David Miller |
Headers | show |
Series | connector: fix unsafe usage of ->real_parent | expand |
Hi 06.03.2019, 09:46, "Li RongQing" <lirongqing@baidu.com>: > proc_exit_connector() uses ->real_parent lockless. This is not > safe that its parent can go away at any moment, so use RCU to > protect it, and ensure that this task is not released. > > Fixes: b086ff87251b4a4 ("connector: add parent pid and tgid to coredump and exit events") > Signed-off-by: Zhang Yu <zhangyu31@baidu.com> > Signed-off-by: Li RongQing <lirongqing@baidu.com> Looks good to me, thank you! Acked-by: Evgeniy Polyakov <zbr@ioremap.net> > --- > drivers/connector/cn_proc.c | 22 ++++++++++++++++++---- > 1 file changed, 18 insertions(+), 4 deletions(-) > > diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c > index ed5e42461094..ad48fd52cb53 100644 > --- a/drivers/connector/cn_proc.c > +++ b/drivers/connector/cn_proc.c > @@ -250,6 +250,7 @@ void proc_coredump_connector(struct task_struct *task) > { > struct cn_msg *msg; > struct proc_event *ev; > + struct task_struct *parent; > __u8 buffer[CN_PROC_MSG_SIZE] __aligned(8); > > if (atomic_read(&proc_event_num_listeners) < 1) > @@ -262,8 +263,14 @@ void proc_coredump_connector(struct task_struct *task) > ev->what = PROC_EVENT_COREDUMP; > ev->event_data.coredump.process_pid = task->pid; > ev->event_data.coredump.process_tgid = task->tgid; > - ev->event_data.coredump.parent_pid = task->real_parent->pid; > - ev->event_data.coredump.parent_tgid = task->real_parent->tgid; > + > + rcu_read_lock(); > + if (pid_alive(task)) { > + parent = rcu_dereference(task->real_parent); > + ev->event_data.coredump.parent_pid = parent->pid; > + ev->event_data.coredump.parent_tgid = parent->tgid; > + } > + rcu_read_unlock(); > > memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id)); > msg->ack = 0; /* not used */ > @@ -276,6 +283,7 @@ void proc_exit_connector(struct task_struct *task) > { > struct cn_msg *msg; > struct proc_event *ev; > + struct task_struct *parent; > __u8 buffer[CN_PROC_MSG_SIZE] __aligned(8); > > if (atomic_read(&proc_event_num_listeners) < 1) > @@ -290,8 +298,14 @@ void proc_exit_connector(struct task_struct *task) > ev->event_data.exit.process_tgid = task->tgid; > ev->event_data.exit.exit_code = task->exit_code; > ev->event_data.exit.exit_signal = task->exit_signal; > - ev->event_data.exit.parent_pid = task->real_parent->pid; > - ev->event_data.exit.parent_tgid = task->real_parent->tgid; > + > + rcu_read_lock(); > + if (pid_alive(task)) { > + parent = rcu_dereference(task->real_parent); > + ev->event_data.exit.parent_pid = parent->pid; > + ev->event_data.exit.parent_tgid = parent->tgid; > + } > + rcu_read_unlock(); > > memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id)); > msg->ack = 0; /* not used */ > -- > 2.16.2
From: Li RongQing <lirongqing@baidu.com> Date: Wed, 6 Mar 2019 14:46:27 +0800 > proc_exit_connector() uses ->real_parent lockless. This is not > safe that its parent can go away at any moment, so use RCU to > protect it, and ensure that this task is not released. ... > Fixes: b086ff87251b4a4 ("connector: add parent pid and tgid to coredump and exit events") > Signed-off-by: Zhang Yu <zhangyu31@baidu.com> > Signed-off-by: Li RongQing <lirongqing@baidu.com> Applied and queued up for -stable, thanks.
diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c index ed5e42461094..ad48fd52cb53 100644 --- a/drivers/connector/cn_proc.c +++ b/drivers/connector/cn_proc.c @@ -250,6 +250,7 @@ void proc_coredump_connector(struct task_struct *task) { struct cn_msg *msg; struct proc_event *ev; + struct task_struct *parent; __u8 buffer[CN_PROC_MSG_SIZE] __aligned(8); if (atomic_read(&proc_event_num_listeners) < 1) @@ -262,8 +263,14 @@ void proc_coredump_connector(struct task_struct *task) ev->what = PROC_EVENT_COREDUMP; ev->event_data.coredump.process_pid = task->pid; ev->event_data.coredump.process_tgid = task->tgid; - ev->event_data.coredump.parent_pid = task->real_parent->pid; - ev->event_data.coredump.parent_tgid = task->real_parent->tgid; + + rcu_read_lock(); + if (pid_alive(task)) { + parent = rcu_dereference(task->real_parent); + ev->event_data.coredump.parent_pid = parent->pid; + ev->event_data.coredump.parent_tgid = parent->tgid; + } + rcu_read_unlock(); memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id)); msg->ack = 0; /* not used */ @@ -276,6 +283,7 @@ void proc_exit_connector(struct task_struct *task) { struct cn_msg *msg; struct proc_event *ev; + struct task_struct *parent; __u8 buffer[CN_PROC_MSG_SIZE] __aligned(8); if (atomic_read(&proc_event_num_listeners) < 1) @@ -290,8 +298,14 @@ void proc_exit_connector(struct task_struct *task) ev->event_data.exit.process_tgid = task->tgid; ev->event_data.exit.exit_code = task->exit_code; ev->event_data.exit.exit_signal = task->exit_signal; - ev->event_data.exit.parent_pid = task->real_parent->pid; - ev->event_data.exit.parent_tgid = task->real_parent->tgid; + + rcu_read_lock(); + if (pid_alive(task)) { + parent = rcu_dereference(task->real_parent); + ev->event_data.exit.parent_pid = parent->pid; + ev->event_data.exit.parent_tgid = parent->tgid; + } + rcu_read_unlock(); memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id)); msg->ack = 0; /* not used */