From 47388e807f85948eefc403a8a5fdc5b406a65d5a Mon Sep 17 00:00:00 2001 From: Daniel Starke Date: Wed, 24 Apr 2024 07:48:41 +0200 Subject: [PATCH 1/5] tty: n_gsm: fix possible out-of-bounds in gsm0_receive() Assuming the following: - side A configures the n_gsm in basic option mode - side B sends the header of a basic option mode frame with data length 1 - side A switches to advanced option mode - side B sends 2 data bytes which exceeds gsm->len Reason: gsm->len is not used in advanced option mode. - side A switches to basic option mode - side B keeps sending until gsm0_receive() writes past gsm->buf Reason: Neither gsm->state nor gsm->len have been reset after reconfiguration. Fix this by changing gsm->count to gsm->len comparison from equal to less than. Also add upper limit checks against the constant MAX_MRU in gsm0_receive() and gsm1_receive() to harden against memory corruption of gsm->len and gsm->mru. All other checks remain as we still need to limit the data according to the user configuration and actual payload size. Reported-by: j51569436@gmail.com Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218708 Tested-by: j51569436@gmail.com Fixes: e1eaea46bb40 ("tty: n_gsm line discipline") Cc: stable@vger.kernel.org Signed-off-by: Daniel Starke Link: https://lore.kernel.org/r/20240424054842.7741-1-daniel.starke@siemens.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/n_gsm.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index 4036566febcb..72b82bf1c280 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -2913,7 +2913,10 @@ static void gsm0_receive(struct gsm_mux *gsm, u8 c) break; case GSM_DATA: /* Data */ gsm->buf[gsm->count++] = c; - if (gsm->count == gsm->len) { + if (gsm->count >= MAX_MRU) { + gsm->bad_size++; + gsm->state = GSM_SEARCH; + } else if (gsm->count >= gsm->len) { /* Calculate final FCS for UI frames over all data */ if ((gsm->control & ~PF) != UIH) { gsm->fcs = gsm_fcs_add_block(gsm->fcs, gsm->buf, @@ -3026,7 +3029,7 @@ static void gsm1_receive(struct gsm_mux *gsm, u8 c) gsm->state = GSM_DATA; break; case GSM_DATA: /* Data */ - if (gsm->count > gsm->mru) { /* Allow one for the FCS */ + if (gsm->count > gsm->mru || gsm->count > MAX_MRU) { /* Allow one for the FCS */ gsm->state = GSM_OVERRUN; gsm->bad_size++; } else From 70d7f1427afcf7fa2d21cb5a04c6f3555d5b9357 Mon Sep 17 00:00:00 2001 From: Daniel Starke Date: Wed, 24 Apr 2024 07:48:42 +0200 Subject: [PATCH 2/5] tty: n_gsm: fix missing receive state reset after mode switch The current implementation uses either gsm0_receive() or gsm1_receive() depending on whether the user configured the mux in basic or advanced option mode. Both functions share some state values over the same logical elements of the frame. However, both frame types differ in their nature. gsm0_receive() uses non-transparency framing, whereas gsm1_receive() uses transparency mechanism. Switching between both modes leaves the receive function in an undefined state when done during frame reception. Fix this by splitting both states. Add gsm0_receive_state_check_and_fix() and gsm1_receive_state_check_and_fix() to ensure that gsm->state is reset after a change of gsm->receive. Note that gsm->state is only accessed in: - gsm0_receive() - gsm1_receive() - gsm_error() Fixes: e1eaea46bb40 ("tty: n_gsm line discipline") Cc: stable@vger.kernel.org Signed-off-by: Daniel Starke Link: https://lore.kernel.org/r/20240424054842.7741-2-daniel.starke@siemens.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/n_gsm.c | 133 ++++++++++++++++++++++++++++++-------------- 1 file changed, 92 insertions(+), 41 deletions(-) diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index 72b82bf1c280..afbf7837b53e 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -245,16 +245,18 @@ enum gsm_encoding { enum gsm_mux_state { GSM_SEARCH, - GSM_START, - GSM_ADDRESS, - GSM_CONTROL, - GSM_LEN, - GSM_DATA, - GSM_FCS, - GSM_OVERRUN, - GSM_LEN0, - GSM_LEN1, - GSM_SSOF, + GSM0_ADDRESS, + GSM0_CONTROL, + GSM0_LEN0, + GSM0_LEN1, + GSM0_DATA, + GSM0_FCS, + GSM0_SSOF, + GSM1_START, + GSM1_ADDRESS, + GSM1_CONTROL, + GSM1_DATA, + GSM1_OVERRUN, }; /* @@ -2847,6 +2849,30 @@ invalid: return; } +/** + * gsm0_receive_state_check_and_fix - check and correct receive state + * @gsm: gsm data for this ldisc instance + * + * Ensures that the current receive state is valid for basic option mode. + */ + +static void gsm0_receive_state_check_and_fix(struct gsm_mux *gsm) +{ + switch (gsm->state) { + case GSM_SEARCH: + case GSM0_ADDRESS: + case GSM0_CONTROL: + case GSM0_LEN0: + case GSM0_LEN1: + case GSM0_DATA: + case GSM0_FCS: + case GSM0_SSOF: + break; + default: + gsm->state = GSM_SEARCH; + break; + } +} /** * gsm0_receive - perform processing for non-transparency @@ -2860,26 +2886,27 @@ static void gsm0_receive(struct gsm_mux *gsm, u8 c) { unsigned int len; + gsm0_receive_state_check_and_fix(gsm); switch (gsm->state) { case GSM_SEARCH: /* SOF marker */ if (c == GSM0_SOF) { - gsm->state = GSM_ADDRESS; + gsm->state = GSM0_ADDRESS; gsm->address = 0; gsm->len = 0; gsm->fcs = INIT_FCS; } break; - case GSM_ADDRESS: /* Address EA */ + case GSM0_ADDRESS: /* Address EA */ gsm->fcs = gsm_fcs_add(gsm->fcs, c); if (gsm_read_ea(&gsm->address, c)) - gsm->state = GSM_CONTROL; + gsm->state = GSM0_CONTROL; break; - case GSM_CONTROL: /* Control Byte */ + case GSM0_CONTROL: /* Control Byte */ gsm->fcs = gsm_fcs_add(gsm->fcs, c); gsm->control = c; - gsm->state = GSM_LEN0; + gsm->state = GSM0_LEN0; break; - case GSM_LEN0: /* Length EA */ + case GSM0_LEN0: /* Length EA */ gsm->fcs = gsm_fcs_add(gsm->fcs, c); if (gsm_read_ea(&gsm->len, c)) { if (gsm->len > gsm->mru) { @@ -2889,14 +2916,14 @@ static void gsm0_receive(struct gsm_mux *gsm, u8 c) } gsm->count = 0; if (!gsm->len) - gsm->state = GSM_FCS; + gsm->state = GSM0_FCS; else - gsm->state = GSM_DATA; + gsm->state = GSM0_DATA; break; } - gsm->state = GSM_LEN1; + gsm->state = GSM0_LEN1; break; - case GSM_LEN1: + case GSM0_LEN1: gsm->fcs = gsm_fcs_add(gsm->fcs, c); len = c; gsm->len |= len << 7; @@ -2907,11 +2934,11 @@ static void gsm0_receive(struct gsm_mux *gsm, u8 c) } gsm->count = 0; if (!gsm->len) - gsm->state = GSM_FCS; + gsm->state = GSM0_FCS; else - gsm->state = GSM_DATA; + gsm->state = GSM0_DATA; break; - case GSM_DATA: /* Data */ + case GSM0_DATA: /* Data */ gsm->buf[gsm->count++] = c; if (gsm->count >= MAX_MRU) { gsm->bad_size++; @@ -2922,14 +2949,14 @@ static void gsm0_receive(struct gsm_mux *gsm, u8 c) gsm->fcs = gsm_fcs_add_block(gsm->fcs, gsm->buf, gsm->count); } - gsm->state = GSM_FCS; + gsm->state = GSM0_FCS; } break; - case GSM_FCS: /* FCS follows the packet */ + case GSM0_FCS: /* FCS follows the packet */ gsm->fcs = gsm_fcs_add(gsm->fcs, c); - gsm->state = GSM_SSOF; + gsm->state = GSM0_SSOF; break; - case GSM_SSOF: + case GSM0_SSOF: gsm->state = GSM_SEARCH; if (c == GSM0_SOF) gsm_queue(gsm); @@ -2942,6 +2969,29 @@ static void gsm0_receive(struct gsm_mux *gsm, u8 c) } } +/** + * gsm1_receive_state_check_and_fix - check and correct receive state + * @gsm: gsm data for this ldisc instance + * + * Ensures that the current receive state is valid for advanced option mode. + */ + +static void gsm1_receive_state_check_and_fix(struct gsm_mux *gsm) +{ + switch (gsm->state) { + case GSM_SEARCH: + case GSM1_START: + case GSM1_ADDRESS: + case GSM1_CONTROL: + case GSM1_DATA: + case GSM1_OVERRUN: + break; + default: + gsm->state = GSM_SEARCH; + break; + } +} + /** * gsm1_receive - perform processing for non-transparency * @gsm: gsm data for this ldisc instance @@ -2952,6 +3002,7 @@ static void gsm0_receive(struct gsm_mux *gsm, u8 c) static void gsm1_receive(struct gsm_mux *gsm, u8 c) { + gsm1_receive_state_check_and_fix(gsm); /* handle XON/XOFF */ if ((c & ISO_IEC_646_MASK) == XON) { gsm->constipated = true; @@ -2964,11 +3015,11 @@ static void gsm1_receive(struct gsm_mux *gsm, u8 c) } if (c == GSM1_SOF) { /* EOF is only valid in frame if we have got to the data state */ - if (gsm->state == GSM_DATA) { + if (gsm->state == GSM1_DATA) { if (gsm->count < 1) { /* Missing FSC */ gsm->malformed++; - gsm->state = GSM_START; + gsm->state = GSM1_START; return; } /* Remove the FCS from data */ @@ -2984,14 +3035,14 @@ static void gsm1_receive(struct gsm_mux *gsm, u8 c) gsm->fcs = gsm_fcs_add(gsm->fcs, gsm->buf[gsm->count]); gsm->len = gsm->count; gsm_queue(gsm); - gsm->state = GSM_START; + gsm->state = GSM1_START; return; } /* Any partial frame was a runt so go back to start */ - if (gsm->state != GSM_START) { + if (gsm->state != GSM1_START) { if (gsm->state != GSM_SEARCH) gsm->malformed++; - gsm->state = GSM_START; + gsm->state = GSM1_START; } /* A SOF in GSM_START means we are still reading idling or framing bytes */ @@ -3012,30 +3063,30 @@ static void gsm1_receive(struct gsm_mux *gsm, u8 c) gsm->escape = false; } switch (gsm->state) { - case GSM_START: /* First byte after SOF */ + case GSM1_START: /* First byte after SOF */ gsm->address = 0; - gsm->state = GSM_ADDRESS; + gsm->state = GSM1_ADDRESS; gsm->fcs = INIT_FCS; fallthrough; - case GSM_ADDRESS: /* Address continuation */ + case GSM1_ADDRESS: /* Address continuation */ gsm->fcs = gsm_fcs_add(gsm->fcs, c); if (gsm_read_ea(&gsm->address, c)) - gsm->state = GSM_CONTROL; + gsm->state = GSM1_CONTROL; break; - case GSM_CONTROL: /* Control Byte */ + case GSM1_CONTROL: /* Control Byte */ gsm->fcs = gsm_fcs_add(gsm->fcs, c); gsm->control = c; gsm->count = 0; - gsm->state = GSM_DATA; + gsm->state = GSM1_DATA; break; - case GSM_DATA: /* Data */ + case GSM1_DATA: /* Data */ if (gsm->count > gsm->mru || gsm->count > MAX_MRU) { /* Allow one for the FCS */ - gsm->state = GSM_OVERRUN; + gsm->state = GSM1_OVERRUN; gsm->bad_size++; } else gsm->buf[gsm->count++] = c; break; - case GSM_OVERRUN: /* Over-long - eg a dropped SOF */ + case GSM1_OVERRUN: /* Over-long - eg a dropped SOF */ break; default: pr_debug("%s: unhandled state: %d\n", __func__, gsm->state); From 4244f830a56058ee0670d80e7ac9fd7c982eb480 Mon Sep 17 00:00:00 2001 From: Pin-yen Lin Date: Wed, 24 Apr 2024 20:58:08 +0800 Subject: [PATCH 3/5] serial: 8520_mtk: Set RTS on shutdown for Rx in-band wakeup When Rx in-band wakeup is enabled, set RTS to true in mtk8250_shutdown() so the connected device can still send message and trigger IRQ when the system is suspended. Fixes: 18c9d4a3c249 ("serial: When UART is suspended, set RTS to false") Cc: stable Signed-off-by: Pin-yen Lin Link: https://lore.kernel.org/r/20240424130619.2924456-1-treapking@chromium.org Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_mtk.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c index 9ff6bbe9c086..d14988d1492f 100644 --- a/drivers/tty/serial/8250/8250_mtk.c +++ b/drivers/tty/serial/8250/8250_mtk.c @@ -209,15 +209,19 @@ static int mtk8250_startup(struct uart_port *port) static void mtk8250_shutdown(struct uart_port *port) { -#ifdef CONFIG_SERIAL_8250_DMA struct uart_8250_port *up = up_to_u8250p(port); struct mtk8250_data *data = port->private_data; + int irq = data->rx_wakeup_irq; +#ifdef CONFIG_SERIAL_8250_DMA if (up->dma) data->rx_status = DMA_RX_SHUTDOWN; #endif - return serial8250_do_shutdown(port); + serial8250_do_shutdown(port); + + if (irq >= 0) + serial8250_do_set_mctrl(&up->port, TIOCM_RTS); } static void mtk8250_disable_intrs(struct uart_8250_port *up, int mask) From 614a19b89ca43449196a8af1afac7d55c6781687 Mon Sep 17 00:00:00 2001 From: Doug Berger Date: Wed, 24 Apr 2024 15:25:59 -0700 Subject: [PATCH 4/5] serial: 8250_bcm7271: use default_mux_rate if possible There is a scenario when resuming from some power saving states with no_console_suspend where console output can be generated before the 8250_bcm7271 driver gets the opportunity to restore the baud_mux_clk frequency. Since the baud_mux_clk is at its default frequency at this time the output can be garbled until the driver gets the opportunity to resume. Since this is only an issue with console use of the serial port during that window and the console isn't likely to use baud rates that require alternate baud_mux_clk frequencies, allow the driver to select the default_mux_rate if it is accurate enough. Fixes: 41a469482de2 ("serial: 8250: Add new 8250-core based Broadcom STB driver") Cc: stable@vger.kernel.org Signed-off-by: Doug Berger Reviewed-by: Florian Fainelli Tested-by: Florian Fainelli Link: https://lore.kernel.org/r/20240424222559.1844045-1-opendmb@gmail.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_bcm7271.c | 99 +++++++++++++++----------- 1 file changed, 59 insertions(+), 40 deletions(-) diff --git a/drivers/tty/serial/8250/8250_bcm7271.c b/drivers/tty/serial/8250/8250_bcm7271.c index 5daa38d9c64e..61d81b11f6a3 100644 --- a/drivers/tty/serial/8250/8250_bcm7271.c +++ b/drivers/tty/serial/8250/8250_bcm7271.c @@ -675,18 +675,46 @@ static void init_real_clk_rates(struct device *dev, struct brcmuart_priv *priv) clk_set_rate(priv->baud_mux_clk, priv->default_mux_rate); } +static u32 find_quot(struct device *dev, u32 freq, u32 baud, u32 *percent) +{ + u32 quot; + u32 rate; + u64 hires_rate; + u64 hires_baud; + u64 hires_err; + + rate = freq / 16; + quot = DIV_ROUND_CLOSEST(rate, baud); + if (!quot) + return 0; + + /* increase resolution to get xx.xx percent */ + hires_rate = div_u64((u64)rate * 10000, (u64)quot); + hires_baud = (u64)baud * 10000; + + /* get the delta */ + if (hires_rate > hires_baud) + hires_err = (hires_rate - hires_baud); + else + hires_err = (hires_baud - hires_rate); + + *percent = (unsigned long)DIV_ROUND_CLOSEST_ULL(hires_err, baud); + + dev_dbg(dev, "Baud rate: %u, MUX Clk: %u, Error: %u.%u%%\n", + baud, freq, *percent / 100, *percent % 100); + + return quot; +} + static void set_clock_mux(struct uart_port *up, struct brcmuart_priv *priv, u32 baud) { u32 percent; u32 best_percent = UINT_MAX; u32 quot; + u32 freq; u32 best_quot = 1; - u32 rate; - int best_index = -1; - u64 hires_rate; - u64 hires_baud; - u64 hires_err; + u32 best_freq = 0; int rc; int i; int real_baud; @@ -695,44 +723,35 @@ static void set_clock_mux(struct uart_port *up, struct brcmuart_priv *priv, if (priv->baud_mux_clk == NULL) return; - /* Find the closest match for specified baud */ - for (i = 0; i < ARRAY_SIZE(priv->real_rates); i++) { - if (priv->real_rates[i] == 0) - continue; - rate = priv->real_rates[i] / 16; - quot = DIV_ROUND_CLOSEST(rate, baud); - if (!quot) - continue; + /* Try default_mux_rate first */ + quot = find_quot(up->dev, priv->default_mux_rate, baud, &percent); + if (quot) { + best_percent = percent; + best_freq = priv->default_mux_rate; + best_quot = quot; + } + /* If more than 1% error, find the closest match for specified baud */ + if (best_percent > 100) { + for (i = 0; i < ARRAY_SIZE(priv->real_rates); i++) { + freq = priv->real_rates[i]; + if (freq == 0 || freq == priv->default_mux_rate) + continue; + quot = find_quot(up->dev, freq, baud, &percent); + if (!quot) + continue; - /* increase resolution to get xx.xx percent */ - hires_rate = (u64)rate * 10000; - hires_baud = (u64)baud * 10000; - - hires_err = div_u64(hires_rate, (u64)quot); - - /* get the delta */ - if (hires_err > hires_baud) - hires_err = (hires_err - hires_baud); - else - hires_err = (hires_baud - hires_err); - - percent = (unsigned long)DIV_ROUND_CLOSEST_ULL(hires_err, baud); - dev_dbg(up->dev, - "Baud rate: %u, MUX Clk: %u, Error: %u.%u%%\n", - baud, priv->real_rates[i], percent / 100, - percent % 100); - if (percent < best_percent) { - best_percent = percent; - best_index = i; - best_quot = quot; + if (percent < best_percent) { + best_percent = percent; + best_freq = freq; + best_quot = quot; + } } } - if (best_index == -1) { + if (!best_freq) { dev_err(up->dev, "Error, %d BAUD rate is too fast.\n", baud); return; } - rate = priv->real_rates[best_index]; - rc = clk_set_rate(priv->baud_mux_clk, rate); + rc = clk_set_rate(priv->baud_mux_clk, best_freq); if (rc) dev_err(up->dev, "Error selecting BAUD MUX clock\n"); @@ -741,8 +760,8 @@ static void set_clock_mux(struct uart_port *up, struct brcmuart_priv *priv, dev_err(up->dev, "Error, baud: %d has %u.%u%% error\n", baud, percent / 100, percent % 100); - real_baud = rate / 16 / best_quot; - dev_dbg(up->dev, "Selecting BAUD MUX rate: %u\n", rate); + real_baud = best_freq / 16 / best_quot; + dev_dbg(up->dev, "Selecting BAUD MUX rate: %u\n", best_freq); dev_dbg(up->dev, "Requested baud: %u, Actual baud: %u\n", baud, real_baud); @@ -751,7 +770,7 @@ static void set_clock_mux(struct uart_port *up, struct brcmuart_priv *priv, i += (i / 2); priv->char_wait = ns_to_ktime(i); - up->uartclk = rate; + up->uartclk = best_freq; } static void brcmstb_set_termios(struct uart_port *up, From 8492bd91aa055907c67ef04f2b56f6dadd1f44bf Mon Sep 17 00:00:00 2001 From: Hugo Villeneuve Date: Tue, 30 Apr 2024 16:04:30 -0400 Subject: [PATCH 5/5] serial: sc16is7xx: fix bug in sc16is7xx_set_baud() when using prescaler When using a high speed clock with a low baud rate, the 4x prescaler is automatically selected if required. In that case, sc16is7xx_set_baud() properly configures the chip registers, but returns an incorrect baud rate by not taking into account the prescaler value. This incorrect baud rate is then fed to uart_update_timeout(). For example, with an input clock of 80MHz, and a selected baud rate of 50, sc16is7xx_set_baud() will return 200 instead of 50. Fix this by first changing the prescaler variable to hold the selected prescaler value instead of the MCR bitfield. Then properly take into account the selected prescaler value in the return value computation. Also add better documentation about the divisor value computation. Fixes: dfeae619d781 ("serial: sc16is7xx") Cc: stable@vger.kernel.org Signed-off-by: Hugo Villeneuve Reviewed-by: Jiri Slaby Link: https://lore.kernel.org/r/20240430200431.4102923-1-hugo@hugovil.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/sc16is7xx.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c index 929206a9a6e1..12915fffac27 100644 --- a/drivers/tty/serial/sc16is7xx.c +++ b/drivers/tty/serial/sc16is7xx.c @@ -554,16 +554,28 @@ static bool sc16is7xx_regmap_noinc(struct device *dev, unsigned int reg) return reg == SC16IS7XX_RHR_REG; } +/* + * Configure programmable baud rate generator (divisor) according to the + * desired baud rate. + * + * From the datasheet, the divisor is computed according to: + * + * XTAL1 input frequency + * ----------------------- + * prescaler + * divisor = --------------------------- + * baud-rate x sampling-rate + */ static int sc16is7xx_set_baud(struct uart_port *port, int baud) { struct sc16is7xx_one *one = to_sc16is7xx_one(port, port); u8 lcr; - u8 prescaler = 0; + unsigned int prescaler = 1; unsigned long clk = port->uartclk, div = clk / 16 / baud; if (div >= BIT(16)) { - prescaler = SC16IS7XX_MCR_CLKSEL_BIT; - div /= 4; + prescaler = 4; + div /= prescaler; } /* Enable enhanced features */ @@ -573,9 +585,10 @@ static int sc16is7xx_set_baud(struct uart_port *port, int baud) SC16IS7XX_EFR_ENABLE_BIT); sc16is7xx_efr_unlock(port); + /* If bit MCR_CLKSEL is set, the divide by 4 prescaler is activated. */ sc16is7xx_port_update(port, SC16IS7XX_MCR_REG, SC16IS7XX_MCR_CLKSEL_BIT, - prescaler); + prescaler == 1 ? 0 : SC16IS7XX_MCR_CLKSEL_BIT); /* Backup LCR and access special register set (DLL/DLH) */ lcr = sc16is7xx_port_read(port, SC16IS7XX_LCR_REG); @@ -591,7 +604,7 @@ static int sc16is7xx_set_baud(struct uart_port *port, int baud) /* Restore LCR and access to general register set */ sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, lcr); - return DIV_ROUND_CLOSEST(clk / 16, div); + return DIV_ROUND_CLOSEST((clk / prescaler) / 16, div); } static void sc16is7xx_handle_rx(struct uart_port *port, unsigned int rxlen,