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

Oleg Nesterov
2008-12-03T19:08:27Z

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.