linux-kernel - Re: atmel_serial might lose modem status change

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3efb10970811121104o7497d354n6730a23ce5756093@mail.gmail.com>
Date:	Wed, 12 Nov 2008 20:04:24 +0100
From:	"Remy Bohmer" <linux@...mer.net>
To:	"Atsushi Nemoto" <anemo@....ocn.ne.jp>
Cc:	hskinnemoen@...el.com, marc.pignat@...s.ch,
	linux-kernel@...r.kernel.org
Subject: Re: atmel_serial might lose modem status change

Hello Atsushi,

2008/11/12 Atsushi Nemoto <anemo@....ocn.ne.jp>:
> I found a problem of handling of modem status of atmel_serial driver.
>
> With the commit 1ecc26 ("atmel_serial: split the interrupt handler"),
> handling of modem status signal was splitted into two parts.  The
> atmel_tasklet_func() compares new status with irq_status_prev, but
> irq_status_prev is not correct if signal status was changed while the
> port is closed.
>
> Here is a sequence to cause problem:
>
> 1. Remote side sets CTS (and DSR).
> 2. Local side close the port.
> 3. Local side clears RTS and DTR.
> 4. Remote side clears CTS and DSR.
> 5. Local side reopen the port.  hw_stopped becomes 1.
> 6. Local side sets RTS and DTR.
> 7. Remote side sets CTS and DSR.
>
> Then CTS change interrupt can be received, but since CTS bit in
> irq_status_prev and new status is same, uart_handle_cts_change() will
> not be called (so hw_stopped will not be cleared, i.e. cannot send any
> data).
>
> I suppose irq_status_prev should be initialized at somewhere in open
> sequence.  Is this fix correct?

I cannot shoot a hole in it... So, to me it appears okay.

Kind Regards,

Remy


> diff --git a/drivers/serial/atmel_serial.c b/drivers/serial/atmel_serial.c
> index 4c535f7..9f3b697 100644
> --- a/drivers/serial/atmel_serial.c
> +++ b/drivers/serial/atmel_serial.c
> @@ -877,6 +877,9 @@ static int atmel_startup(struct uart_port *port)
>                }
>        }
>
> +       /* Save current CSR for comparison in atmel_tasklet_func() */
> +       atmel_port->irq_status_prev = UART_GET_CSR(port);
> +
>        /*
>         * Finally, enable the serial port
>         */
>
> ---
> Atsushi Nemoto
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/