Messages in this thread | | | Date | Tue, 08 Aug 2006 19:53:01 +0400 | From | Kirill Korotaev <> | Subject | Re: [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/
| |