[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 08 Aug 2006 19:53:01 +0400
From: Kirill Korotaev <dev@...ru>
To: Eric Dumazet <dada1@...mosbay.com>
CC: Andrew Morton <akpm@...l.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
xemul@...ru, hch@...radead.org
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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists