Re: BUG? "Call fasync() functions without the BKL" is racy

[Oleg Nesterov <oleg@redhat.com>]

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.