Re: pty_chars_in_buffer NULL pointer (kernel oops)

[Linus Torvalds]

On Sun, 27 Feb 2005, Marcelo Tosatti wrote:
> 
> Alan, Linus, what correction to the which the above thread discusses has 
> been deployed? 

This is the hacky "hide the problem" patch that is in my current tree (and 
was discussed in the original thread some time ago).

It's in no way "correct", in that the race hasn't actually gone away by 
this patch, but the patch makes it unimportant. We may end up calling a 
stale line discipline, which is still very wrong, but it so happens that 
we don't much care in practice.

I think that in a 2.4.x tree there are some theoretical SMP races with
module unloading etc (which the 2.6.x code doesn't have because module
unload stops the other CPU's - maybe that part got backported to 2.4.x?), 
but quite frankly, I suspect that even in 2.4.x they are entirely 
theoretical and impossible to actually hit.

And again, in theory some line discipline might do something strange in
it's "chars_in_buffer" routine that would be problematic. In practice
that's just not the case: the "chars_in_buffer()" routine might return a
bogus _value_ for a stale line discipline thing, but none of them seem to
follow any pointers that might have become invalid (and in fact, most
ldiscs don't even have that function).

So while this patch is wrogn in theory, it does have the advantage of
being (a) very safe minimal patch and (b) fixing the problem in practice
with no performance downside.

I still feel a bit guilty about it, though.

			Linus

---
# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2005/02/25 19:39:39-08:00 torvalds@xxxxxxxxxxxxxxx 
#   Fix possible pty line discipline race.
#   
#   This ain't pretty. Real fix under discussion.
# 
# drivers/char/pty.c
#   2005/02/25 19:39:32-08:00 torvalds@xxxxxxxxxxxxxxx +4 -2
#   Fix possible pty line discipline race.
#   
#   This ain't pretty. Real fix under discussion.
# 
diff -Nru a/drivers/char/pty.c b/drivers/char/pty.c
--- a/drivers/char/pty.c	2005-02-27 11:31:57 -08:00
+++ b/drivers/char/pty.c	2005-02-27 11:31:57 -08:00
@@ -149,13 +149,15 @@
 static int pty_chars_in_buffer(struct tty_struct *tty)
 {
 	struct tty_struct *to = tty->link;
+	ssize_t (*chars_in_buffer)(struct tty_struct *);
 	int count;
 
-	if (!to || !to->ldisc.chars_in_buffer)
+	/* We should get the line discipline lock for "tty->link" */
+	if (!to || !(chars_in_buffer = to->ldisc.chars_in_buffer))
 		return 0;
 
 	/* The ldisc must report 0 if no characters available to be read */
-	count = to->ldisc.chars_in_buffer(to);
+	count = chars_in_buffer(to);
 
 	if (tty->driver->subtype == PTY_TYPE_SLAVE) return count;
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/