Re: [PATCH] unserialized task->files changing (v2)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux