Re: [PATCH] race condition with drivers/char/vt.c (bug invt_ioctl.c)

From: Steven Rostedt
Date: Mon Aug 22 2005 - 16:26:03 EST


On Mon, 2005-08-22 at 09:13 +0200, Ingo Molnar wrote:

>
> cool fix. I'm wondering, there's a whole lot of other 'tty->count == 1'
> checks in drivers/char/*.c, could some of those be racy too?

I checked them out. The main problem is that tty->count == 1 is not
reliable in the open function call. Of the 22 files that use tty->count
== 1, only 4 of them (not including vt.c) have a potential race.

Three of the 4, have the race only if an error occurred.

synclink.c
synclink_cs.c
synclinkmp.c

The code in these three have something like this:


static int open(struct tty_struct *tty, struct file *filp)
{
...
if (/* some error */) {
retval = /* something bad */
goto cleanup;
}
...

cleanup:
if (retval) {
if (tty->count == 1)
info->tty = NULL; /* tty layer will release tty struct */
if(info->count)
info->count--;
}

return retval;
}

So, the info->tty has a chance of not being set to NULL. I don't know
how bad that is, but this is probably a bug.

The fourth file "ip2main.c" is a little more of a problem.

It's open has the following code:

noblock:

/* first open - Assign termios structure to port */
if ( tty->count == 1 ) {
i2QueueCommands(PTYPE_INLINE, pCh, 0, 2, CMD_CTSFL_DSAB, CMD_RTSFL_DSAB);
/* Now we must send the termios settings to the loadware */
set_params( pCh, NULL );
}

So, there's a chance that tty->count does not equal 1 here when it
should. The way to fix this would probably be to set driver_data to
NULL with some locking around it, and check that instead. Like what was
done for vt.c. Since I don't have an ip2 device that I know of, I won't
be fixing this code.

-- Steve


-
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/