lkml.org 
[lkml]   [2006]   [Aug]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] unserialized task->files changing (v2)
Eric,

> Sorry but there is something I dont understand. You ignored my point.
Sorry, I missed it thinking that you are talking about another thing...
Pavel described the race in more details and why barrier doesn't help.
Hope, it became more clear now.

> +void reset_files_struct(struct task_struct *tsk, struct files_struct *files)
> +{
> + struct files_struct *old;
> +
> + old = tsk->files;
> + task_lock(tsk);
> + tsk->files = files;
> + task_unlock(tsk);
> + put_files_struct(old);
> +}
>
> Its seems very strange to protect tsk->files = files with a
> task_lock()/task_unlock(). What is it supposed to guard against ???
>
> If this patch corrects the 'bug', then a simpler fix would be to use a memory
> barrier between "tsk->files = files" and "put_files_struct(old);"
>
> No need to perform 2 atomics ops on the task lock.
>
> old = tsk->files;
> tsk->files = files;
> smp_mb();
> put_files_struct(old);
>
> That would be enough to guard against proc code (because this code only needs
> to read tsk->files of course)
>
> The same remark can be said for __exit_files() from kernel/exit.c
>
> If this task_lock()/task_unlock() patch is really needed, then a comment in
> the source would be very fair.

Kirill
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-08-08 17:55    [W:0.026 / U:4.900 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site