On Wed, 16 Feb 2011 17:44:27 -0800
"Mills, Ken K" <[email protected]> wrote:

> From: Ken Mills <[email protected]>
>               tty_kref_put(tty);
> +             tty->driver_data = 0;
>       }

At the point you dropped the last kref the tty may well have been
deleted so this would scribble into free memory.

If you need something to absolutely definitely happen take a look at
the tty->ops->cleanup() method which is called just before the tty
structure is freed. The tty_port has a similar hook
port->ops->shutdown() which is called when the final user of a given
port closes it, and port->ops->destructor() allows you to override the
release of the port (the default behaviour is kfree(port)).

Note that port->ops->destructor() may be called from IRQ context.
port->ops->shutdown is called from sleeping context on the hangup/close
that drops the usage count to zero, and under the port mutex lock.

>       del_timer_sync(&dlci->t1);
>       dlci->gsm->dlci[dlci->addr] = NULL;
> @@ -2794,6 +2795,8 @@ static void gsmtty_hangup(struct tty_struct
> *tty) {
>       struct gsm_dlci *dlci = tty->driver_data;
>       tty_port_hangup(&dlci->port);
> +     gsm_destroy_network(dlci);
> +     tty->driver_data=0;

You depend upon tty->driver_data after you do this (think about a
parallel hangup and write)

>       gsm_dlci_begin_close(dlci);
>  }
>  
> @@ -2874,6 +2877,8 @@ static int gsmtty_ioctl(struct tty_struct *tty,
> struct file *filp, return -EFAULT;
>               nc.if_name[IFNAMSIZ-1] = '\0';
>               /* return net interface index or error code */
> +             if(dlci == NULL)
> +                     return -EFAULT;

NULL can be a valid user space address - if it isn't then copy_*_user
already handles it so the check isn't needed. Also btw your code is
buggy here too as the ioctl method can be called in parallel by
multiple threads so needs locking.

_______________________________________________
MeeGo-kernel mailing list
[email protected]
http://lists.meego.com/listinfo/meego-kernel

Reply via email to