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

[Steven Rostedt <rostedt@goodmis.org>]

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