Re: [PATCH] console close race fix resend

Gordon Oliver (gordo@pincoya.com)
Tue, 11 Dec 2001 00:54:11 -0800


On 2001.12.10 22:05 Robert Love wrote:
> Ehh, I don't think so. Here is the whole patched function:
>
> static void con_flush_chars(struct tty_struct *tty)
> {
> struct vt_struct *vt = (struct vt_struct *)tty->driver_data;
> if (in_interrupt()) /* from flush_to_ldisc */
> return;
> pm_access(pm_con);
> acquire_console_sem();
> if (vt)
> set_cursor(vt->vc_num);
> release_console_sem();
> }
>
> When we check vt, it isn't stale. vt is a _pointer_ to the data so that
> first reference against it is guaranteed to grab the correct value. The
> only possible race is between the if and the set_cursor, but that isn't
> an issue because we acquired the console semaphore. There is no race
> here.

I like the patch that Andrew Morton sent in reply to this better.
Note that in the event that the above code does the following sequence
it will cause a stale pointer to be used:

con_flush_chars con_close
vt = <>
tty->driver_data = NULL
acquire_console_sem()
set_cursor()
release_console_sem()

Now it _might_ be ok to act on a stale vt pointer, but it sure
feels like thin ice. I'm not sure that there is any danger of
the vt data being modified in a way that would break this, but
since the tty no longer has a reference it is bad practice.

What the earlier patch did is created some very subtle semantics
for a small window of a race. It fixed the blaring bug (the OOPS)
but left a possible one that would be harder to find....
-gordo
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/