lkml.org 
[lkml]   [2013]   [Dec]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] drm/i915: Fix refcount leak and possible NULL pointer dereference.
From
Date
Hello.

Chris Wilson wrote:
> > Since get_pid_task() grabs a reference on the task_struct, we have to drop the
> > refcount after reading that task\'s comm name. Also, directly reading like
> > get_pid_task()->comm can trigger an oops when get_pid_task() returned NULL.
>
> The second issue is moot as file itself cannot exist if the task_struct
> is NULL, and the task_struct cannot be destroyed until we finish the
> function. The simpler fix would appear to be s/get_pid_task/pid_task/

I didn\'t catch. Please see below sample module.

pid_task() will return NULL if the target task has already died.
The target task could be killed (e.g. by the OOM killer) at any time, and
whether the target task\'s task_struct is not yet destroyed is irrelevant
for pid_task().

get_pid_task(file->pid, PIDTYPE_PID)->comm by chance didn\'t trigger
oops because offsetof(struct task_struct, comm) < PAGE_SIZE is true.

---------- sample start ----------
#include <linux/module.h>
#include <linux/delay.h>
#include <linux/kthread.h>

static int test_thread(void *unused)
{
return 0;
}

static int __init test_init(void)
{
struct task_struct *task = kthread_create(test_thread, NULL, \"test\");
if (!IS_ERR(task)) {
struct pid *pid;
struct task_struct *t;
get_task_struct(task);
pid = get_pid(task->pids[PIDTYPE_PID].pid);
rcu_read_lock();
t = pid_task(pid, PIDTYPE_PID);
rcu_read_unlock();
printk(\"pid_task(%p)=%p\\n\", pid, t);
wake_up_process(task);
ssleep(1);
rcu_read_lock();
t = pid_task(pid, PIDTYPE_PID);
rcu_read_unlock();
printk(\"pid_task(%p)=%p\\n\", pid, t);
put_task_struct(task);
put_pid(pid);
}
return -EINVAL;
}

module_init(test_init);
MODULE_LICENSE(\"GPL\");
---------- sample end ----------

---------- output start ----------
pid_task(ffff8800474bff80)=ffff8800435b0e10
pid_task(ffff8800474bff80)= (null)
---------- output end ----------


\
 
 \ /
  Last update: 2013-12-26 02:21    [W:0.091 / U:0.332 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site