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] [day] [month] [year] [list]
Date:   Wed, 5 Dec 2018 01:41:52 +0000
From:   Darwin Dingel <Darwin.Dingel@...iedtelesis.co.nz>
To:     "andriy.shevchenko@...ux.intel.com" 
        <andriy.shevchenko@...ux.intel.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "jslaby@...e.com" <jslaby@...e.com>,
        "linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/2] serial: 8250: Rate limit serial port rx interrupts
 during input overruns

So sorry for the premature "Reviewed-by" bits on the commit messages on the previous patches
submitted. Please ignore these while I fix the patch up for v3.


Regards,
Darwin
________________________________________
From: Darwin Dingel <darwin.dingel@...iedtelesis.co.nz>
Sent: Wednesday, 5 December 2018 1:50 p.m.
To: andriy.shevchenko@...ux.intel.com; gregkh@...uxfoundation.org; jslaby@...e.com; linux-serial@...r.kernel.org
Cc: linux-kernel@...r.kernel.org; Darwin Dingel
Subject: [PATCH v2 2/2] serial: 8250: Rate limit serial port rx interrupts during input overruns

When a serial port gets faulty or gets flooded with inputs, its interrupt
handler starts to work double time to get the characters to the workqueue
for the tty layer to handle them. When this busy time on the serial/tty
subsystem happens during boot, where it is also busy on the userspace
trying to initialise, some processes can continuously get preempted
and will be on hold until the interrupts subside.

The fix is to backoff on processing received characters for a specified
amount of time when an input overrun is seen (received a new character
before the previous one is processed). This only stops receive and will
continue to transmit characters to serial port. After the backoff period
is done, it receive will be re-enabled. This is optional and will only
be enabled by setting 'overrun-throttle-ms' in the dts.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Reviewed-by: Jiri Slaby <jslaby@...e.com>
Signed-off-by: Darwin Dingel <darwin.dingel@...iedtelesis.co.nz>
---
 drivers/tty/serial/8250/8250_core.c | 25 +++++++++++++++++++++++++
 drivers/tty/serial/8250/8250_fsl.c  | 23 ++++++++++++++++++++++-
 drivers/tty/serial/8250/8250_of.c   |  5 +++++
 include/linux/serial_8250.h         |  4 ++++
 4 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 94f3e1c64490..189ab1212d9a 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -942,6 +942,21 @@ static struct uart_8250_port *serial8250_find_match_or_unused(struct uart_port *
        return NULL;
 }

+static void serial_8250_overrun_backoff_work(struct work_struct *work)
+{
+       struct uart_8250_port *up =
+           container_of(to_delayed_work(work), struct uart_8250_port,
+                        overrun_backoff);
+       struct uart_port *port = &up->port;
+       unsigned long flags;
+
+       spin_lock_irqsave(&port->lock, flags);
+       up->ier |= UART_IER_RLSI | UART_IER_RDI;
+       up->port.read_status_mask |= UART_LSR_DR;
+       serial_out(up, UART_IER, up->ier);
+       spin_unlock_irqrestore(&port->lock, flags);
+}
+
 /**
  *     serial8250_register_8250_port - register a serial port
  *     @up: serial port template
@@ -1056,6 +1071,16 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
                        ret = 0;
                }
        }
+
+       /* Initialise interrupt backoff work if required */
+       if (up->overrun_backoff_time_ms > 0) {
+               uart->overrun_backoff_time_ms = up->overrun_backoff_time_ms;
+               INIT_DELAYED_WORK(&uart->overrun_backoff,
+                                 serial_8250_overrun_backoff_work);
+       } else {
+               uart->overrun_backoff_time_ms = 0;
+       }
+
        mutex_unlock(&serial_mutex);

        return ret;
diff --git a/drivers/tty/serial/8250/8250_fsl.c b/drivers/tty/serial/8250/8250_fsl.c
index 6640a4c7ddd1..bb9571eed275 100644
--- a/drivers/tty/serial/8250/8250_fsl.c
+++ b/drivers/tty/serial/8250/8250_fsl.c
@@ -45,8 +45,29 @@ int fsl8250_handle_irq(struct uart_port *port)

        lsr = orig_lsr = up->port.serial_in(&up->port, UART_LSR);

-       if (lsr & (UART_LSR_DR | UART_LSR_BI))
+       /* Process incoming characters first */
+       if ((lsr & (UART_LSR_DR | UART_LSR_BI)) &&
+           (up->ier & (UART_IER_RLSI | UART_IER_RDI))) {
                lsr = serial8250_rx_chars(up, lsr);
+       }
+
+       /* Stop processing interrupts on input overrun */
+       if ((orig_lsr & UART_LSR_OE) && (up->overrun_backoff_time_ms > 0)) {
+               unsigned long delay;
+
+               up->ier = port->serial_in(port, UART_IER);
+               if (up->ier & (UART_IER_RLSI | UART_IER_RDI)) {
+                       port->ops->stop_rx(port);
+               } else {
+                       /* Keep restarting the timer until
+                        * the input overrun subsides.
+                        */
+                       cancel_delayed_work(&up->overrun_backoff);
+               }
+
+               delay = msecs_to_jiffies(up->overrun_backoff_time_ms);
+               schedule_delayed_work(&up->overrun_backoff, delay);
+       }

        serial8250_modem_status(up);

diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
index 877fd7f8a8ed..a1a85805d010 100644
--- a/drivers/tty/serial/8250/8250_of.c
+++ b/drivers/tty/serial/8250/8250_of.c
@@ -240,6 +240,11 @@ static int of_platform_serial_probe(struct platform_device *ofdev)
        if (of_property_read_bool(ofdev->dev.of_node, "auto-flow-control"))
                port8250.capabilities |= UART_CAP_AFE;

+       if (of_property_read_u32(ofdev->dev.of_node,
+                       "overrun-throttle-ms",
+                       &port8250.overrun_backoff_time_ms) != 0)
+               port8250.overrun_backoff_time_ms = 0;
+
        ret = serial8250_register_8250_port(&port8250);
        if (ret < 0)
                goto err_dispose;
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index 18e21427bce4..5a655ba8d273 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -134,6 +134,10 @@ struct uart_8250_port {
        void                    (*dl_write)(struct uart_8250_port *, int);

        struct uart_8250_em485 *em485;
+
+       /* Serial port overrun backoff */
+       struct delayed_work overrun_backoff;
+       u32 overrun_backoff_time_ms;
 };

 static inline struct uart_8250_port *up_to_u8250p(struct uart_port *up)
--
2.19.2

changes in v2:
- separated dts binding to another patch

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ