Re: [patch] disable_bh/enable_bh race fix [Re: Program to freeze keyboard in 2.1.131]

Linus Torvalds (torvalds@transmeta.com)
Sun, 27 Dec 1998 14:37:51 -0800 (PST)


On Sun, 27 Dec 1998, Andrea Arcangeli wrote:
>
> The synchronize_bh doesn't synchronize if it's run from an irq handler so
> we could consider the irq case a special not bh_atomic_safe case? Or
> should we go always safe using a spinlock and do many __cli()?

You missed the basic problem:

CPU#1 CPU#2

start_bh_atomic() start_bh_atomic()
access bh count access bh count
end_bh_atomic() end_bh_atomic()

You can have both processes do the access in parallel with your patch #2.

Your first patch was ok - even if they do the bh disable in parallel, at
least the bh count is sane because it's updated with an atomic read. The
bh masks are another problem, and that should be considered a separate
problem.

Your second patch is bad: it only fixes the specific race in the console
driver, where the other CPU is in a bh context. If the other CPU is _not_
in a bh context, then "start_bh_atomic()" doesn't do anything at all for
you.

A "start_bh_atomic()" does NOT imply any actual critical region locking.
It only means that a bottom half will not run (as the name implies). Two
CPU's can be in different bh-atomic regions at the same time, it _only_
protects against other software interrupt handlers running concurrently.

Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/