Messages in this thread | | | Date | Wed, 3 Dec 2008 20:06:48 +0100 | From | Oleg Nesterov <> | Subject | Re: BUG? "Call fasync() functions without the BKL" is racy |
| |
On 12/02, Andi Kleen wrote: > >> Still I'd like to see the better fix for the long term, the only (afaics) >> flag with the "side effect" is FASYNC, perhaps we can move it to (say) >> ->f_mode, but this is ugly of course and still need serialization for the >> pathes which play with FASYNC. > > I wonder if we need FASYNC at all. This could be gotten implicitely by > looking at the fasync_list
Only if socket.
Serioulsy, I think the best (partial, yes) fix for now is to restore lock_kernel() in setfl() and change ioctl_fioxxx() accordingly. At least this protect us from tty too.
I agree with Jonathan, we need the lock to protect ->f_flags, but this needs changes. Personally, I do not like the global mutex, perhaps we can use some "unused" bit in struct file. Say, ->f_mode is never changed (afaics), we can place it here. Now, any code which changes ->f_flags can do
if (test_and_set_bit(FLAGS_LOCK_BIT)) return;
whatever();
->f_flags = new_flags;
clear_bit(FLAGS_LOCK_BIT);
No need to disable preemption, we never spin waiting for the lock bit. If it is locked - somebody else updates ->f_flags, we can pretend it does this after us. This can confuse F_GETFL after F_SETFL (if F_SETFL "fails"), but I think in that case user-space is wrong anyway, it must not do F_GETFL in parallel.
Not that I think this is very good idea though ;)
Oleg.
| |