On Wed, Feb 27, 2013 at 01:52:27PM +0100, Johan Hovold wrote:
> Make sure to check the serial disconnected flag before accessing port
> private data after waking up.
>
> This fixes a use after free in the ftdi_sio introduced by commit
> 876ae50d94b ("USB: ftdi_sio: fix race condition in TIOCMIWAIT, and abort
> of TIOCMIWAIT when the device is removed").
>
> When switching to tty ports, some lifetime assumptions where changed.
> Specifically, close can now be called before the final tty reference is
> dropped as part of hangup at device disconnect. Even with the ftdi
> private-data refcounting this means that the port private data can be
> freed while a process is sleeping on modem-status changes and thus
> cannot be relied on to detect disconnects when woken up.
Greg, those changed life-times introduced a second use after free here
as well: the wait queue itself. This affects all usb-serial drivers with
private wait queues.
My third series with the TIOCMIWAIT-rework fixes this problem, but
I'll submit something that can more easily be backported to stable
first.
Can you hold back this patch and the two follow up series (or if you
prefer all three of my USB-series) and I'll respin and resubmit them
shortly?
Thanks,
Johan
> Cc: stable <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Johan Hovold <jhovold@xxxxxxxxx>
> ---
> drivers/usb/serial/ftdi_sio.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> index edd162d..83dd0b9 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -71,7 +71,6 @@ struct ftdi_private {
> struct async_icount icount;
> wait_queue_head_t delta_msr_wait; /* Used for TIOCMIWAIT */
> char prev_status; /* Used for TIOCMIWAIT */
> - bool dev_gone; /* Used to abort TIOCMIWAIT */
> char transmit_empty; /* If transmitter is empty or not */
> __u16 interface; /* FT2232C, FT2232H or FT4232H port interface
> (0 for FT232/245) */
> @@ -1694,7 +1693,6 @@ static int ftdi_sio_port_probe(struct usb_serial_port *port)
> init_waitqueue_head(&priv->delta_msr_wait);
>
> priv->flags = ASYNC_LOW_LATENCY;
> - priv->dev_gone = false;
>
> if (quirk && quirk->port_probe)
> quirk->port_probe(priv);
> @@ -1840,7 +1838,6 @@ static int ftdi_sio_port_remove(struct usb_serial_port *port)
> {
> struct ftdi_private *priv = usb_get_serial_port_data(port);
>
> - priv->dev_gone = true;
> wake_up_interruptible_all(&priv->delta_msr_wait);
>
> remove_sysfs_attrs(port);
> @@ -2440,11 +2437,15 @@ static int ftdi_ioctl(struct tty_struct *tty,
> */
> case TIOCMIWAIT:
> cprev = priv->icount;
> - while (!priv->dev_gone) {
> + for (;;) {
> interruptible_sleep_on(&priv->delta_msr_wait);
> /* see if a signal did it */
> if (signal_pending(current))
> return -ERESTARTSYS;
> +
> + if (port->serial->disconnected)
> + return -EIO;
> +
> cnow = priv->icount;
> if (((arg & TIOCM_RNG) && (cnow.rng != cprev.rng)) ||
> ((arg & TIOCM_DSR) && (cnow.dsr != cprev.dsr)) ||
> @@ -2454,8 +2455,6 @@ static int ftdi_ioctl(struct tty_struct *tty,
> }
> cprev = cnow;
> }
> - return -EIO;
> - break;
> case TIOCSERGETLSR:
> return get_lsr_info(port, (struct serial_struct __user *)arg);
> break;
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html