diff for duplicates of <20260607095735.583551579@linuxfoundation.org> diff --git a/a/1.txt b/N1/1.txt index e22c7b0..4bdd4b2 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,68 +1,80 @@ -6.18-stable review patch. If anyone has any objections, please let me know. +6.12-stable review patch. If anyone has any objections, please let me know. ------------------ -From: Ian Abbott <abbotti@mev.co.uk> +From: Tudor Ambarus <tudor.ambarus@linaro.org> -commit 8a3bee801d420be8a7a0bae4a26547b353b8fe22 upstream. +commit a3bb136bff5e6a5e48cdd813246c9c4686feaaa9 upstream. -The function checks and possibly modifies the description of an -asynchronous command to be run on the analog input subdevice of a comedi -device attached to the "comedi_test" driver, returning 0 if no -modifications were required, or a positive value that indicates which -step of the checking process it failed on. Step 4 fixes up various -argument values for various trigger sources. +Sashiko identified a deadlock when the console flow is engaged [1]. -There are two bugs in the fixing up of the `convert_arg` value to keep -the `scan_begin_arg` value within the range of `unsigned int` when -`scan_begin_src` and `convert_src` both have the value `TRIG_TIMER`, -which indicates that the corresponding `_arg` values hold a time period -in nanoseconds. The code also uses `scan_end_arg` which hold the number -of "conversions" within each "scan". The goal is to end up with the -scan period being less than or equal to the convert period multiplied by -the number of conversions per scan. It intends to do that by clamping -the `convert_arg` value to a maximum value of `UINT_MAX / scan_end_arg` -rounded down to a multiple of 1000 (`NSEC_PER_USEC`). +When console flow control is enabled (UPF_CONS_FLOW), +s3c24xx_serial_stop_tx() calls s3c24xx_serial_rx_enable() and +s3c24xx_serial_start_tx() calls s3c24xx_serial_rx_disable(). -(The rounding from nanoseconds to microseconds is because the driver is -modelling a device that uses a 1 MHz clock for timing. This is partly -because that is a more typical timing base for real hardware devices -driven by comedi, and partly because the driver used to use `struct -timeval` internally.) +The serial core framework invokes the .stop_tx() and .start_tx() +callbacks with the port->lock spinlock already held. Furthermore, all +internal driver paths that invoke stop_tx (such as the DMA TX +completion handler s3c24xx_serial_tx_dma_complete() or the PIO TX IRQ +handler s3c24xx_serial_tx_irq()) also acquire port->lock prior to +calling it. (Note that s3c24xx_serial_start_tx() is only invoked by the +serial core). -The first bug is that the code checks if `scan_begin_arg == TRIG_TIMER` -when it should be checking if `scan_begin_src == TRIG_TIMER`. The -bugged check will always fail because if `scan_begin_src == TRIG_TIMER`, -then `scan_begin_arg` will be at least 1000 (`NSEC_PER_USEC`), otherwise -`scan_begin_src == TRIG_FOLLOW` and `scan_begin_arg` will be 0. (N.B -`TRIG_TIMER` is defined as `0x10`.) The second bug is that is rounding -the maximum value down to a multiple of 1000000000 (`NSEC_PER_SEC`) -instead of 1000 (`NSEC_PER_USEC`), however this bug is not reached due -to the first bug. This patch fixes both bugs. +However, s3c24xx_serial_rx_enable() and s3c24xx_serial_rx_disable() +unconditionally attempt to acquire port->lock again using +uart_port_lock_irqsave(). Since spinlocks are not recursive, this +causes a deadlock on the same CPU when console flow control is engaged. + +Remove the redundant lock acquisition from both rx helper functions. -Fixes: 783ddaebd397 ("staging: comedi: comedi_test: support scan_begin_src == TRIG_FOLLOW") -Fixes: 5afdcad2f818 ("staging: comedi: comedi_test: limit maximum convert_arg") Cc: stable <stable@kernel.org> -Signed-off-by: Ian Abbott <abbotti@mev.co.uk> -Link: https://patch.msgid.link/20260422144637.27692-1-abbotti@mev.co.uk +Fixes: b497549a035c ("[ARM] S3C24XX: Split serial driver into core and per-cpu drivers") +Reported-by: John Ogness <john.ogness@linutronix.de> +Closes: https://sashiko.dev/#/patchset/20260506121606.5805-1-john.ogness%40linutronix.de [1] +Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> +Link: https://patch.msgid.link/20260515-samsung-tty-flow-control-deadlock-v1-1-93255edbc9bc@linaro.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- - drivers/comedi/drivers/comedi_test.c | 4 ++-- - 1 file changed, 2 insertions(+), 2 deletions(-) + drivers/tty/serial/samsung_tty.c | 8 -------- + 1 file changed, 8 deletions(-) ---- a/drivers/comedi/drivers/comedi_test.c -+++ b/drivers/comedi/drivers/comedi_test.c -@@ -325,10 +325,10 @@ static int waveform_ai_cmdtest(struct co - arg = min(arg, - rounddown(UINT_MAX, (unsigned int)NSEC_PER_USEC)); - arg = NSEC_PER_USEC * DIV_ROUND_CLOSEST(arg, NSEC_PER_USEC); -- if (cmd->scan_begin_arg == TRIG_TIMER) { -+ if (cmd->scan_begin_src == TRIG_TIMER) { - /* limit convert_arg to keep scan_begin_arg in range */ - limit = UINT_MAX / cmd->scan_end_arg; -- limit = rounddown(limit, (unsigned int)NSEC_PER_SEC); -+ limit = rounddown(limit, (unsigned int)NSEC_PER_USEC); - arg = min(arg, limit); - } - err |= comedi_check_trigger_arg_is(&cmd->convert_arg, arg); +--- a/drivers/tty/serial/samsung_tty.c ++++ b/drivers/tty/serial/samsung_tty.c +@@ -243,12 +243,9 @@ static bool s3c24xx_serial_txempty_nofif + static void s3c24xx_serial_rx_enable(struct uart_port *port) + { + struct s3c24xx_uart_port *ourport = to_ourport(port); +- unsigned long flags; + int count = 10000; + u32 ucon, ufcon; + +- uart_port_lock_irqsave(port, &flags); +- + while (--count && !s3c24xx_serial_txempty_nofifo(port)) + udelay(100); + +@@ -261,23 +258,18 @@ static void s3c24xx_serial_rx_enable(str + wr_regl(port, S3C2410_UCON, ucon); + + ourport->rx_enabled = 1; +- uart_port_unlock_irqrestore(port, flags); + } + + static void s3c24xx_serial_rx_disable(struct uart_port *port) + { + struct s3c24xx_uart_port *ourport = to_ourport(port); +- unsigned long flags; + u32 ucon; + +- uart_port_lock_irqsave(port, &flags); +- + ucon = rd_regl(port, S3C2410_UCON); + ucon &= ~S3C2410_UCON_RXIRQMODE; + wr_regl(port, S3C2410_UCON, ucon); + + ourport->rx_enabled = 0; +- uart_port_unlock_irqrestore(port, flags); + } + + static void s3c24xx_serial_stop_tx(struct uart_port *port) diff --git a/a/content_digest b/N1/content_digest index 7442d56..45d2aa4 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -1,81 +1,94 @@ - "ref\020260607095727.528828913@linuxfoundation.org\0" + "ref\020260607095727.647295505@linuxfoundation.org\0" "From\0Greg Kroah-Hartman <gregkh@linuxfoundation.org>\0" - "Subject\0[PATCH 6.18 218/315] comedi: comedi_test: Fix limiting of convert_arg in waveform_ai_cmdtest()\0" - "Date\0Sun, 7 Jun 2026 12:00:05 +0200\0" + "Subject\0[PATCH 6.12 214/307] tty: serial: samsung: Remove redundant port lock acquisition in rx helpers\0" + "Date\0Sun, 7 Jun 2026 12:00:11 +0200\0" "To\0stable@vger.kernel.org\0" "Cc\0Greg Kroah-Hartman <gregkh@linuxfoundation.org>" patches@lists.linux.dev stable <stable@kernel.org> - " Ian Abbott <abbotti@mev.co.uk>\0" + John Ogness <john.ogness@linutronix.de> + " Tudor Ambarus <tudor.ambarus@linaro.org>\0" "\00:1\0" "b\0" - "6.18-stable review patch. If anyone has any objections, please let me know.\n" + "6.12-stable review patch. If anyone has any objections, please let me know.\n" "\n" "------------------\n" "\n" - "From: Ian Abbott <abbotti@mev.co.uk>\n" + "From: Tudor Ambarus <tudor.ambarus@linaro.org>\n" "\n" - "commit 8a3bee801d420be8a7a0bae4a26547b353b8fe22 upstream.\n" + "commit a3bb136bff5e6a5e48cdd813246c9c4686feaaa9 upstream.\n" "\n" - "The function checks and possibly modifies the description of an\n" - "asynchronous command to be run on the analog input subdevice of a comedi\n" - "device attached to the \"comedi_test\" driver, returning 0 if no\n" - "modifications were required, or a positive value that indicates which\n" - "step of the checking process it failed on. Step 4 fixes up various\n" - "argument values for various trigger sources.\n" + "Sashiko identified a deadlock when the console flow is engaged [1].\n" "\n" - "There are two bugs in the fixing up of the `convert_arg` value to keep\n" - "the `scan_begin_arg` value within the range of `unsigned int` when\n" - "`scan_begin_src` and `convert_src` both have the value `TRIG_TIMER`,\n" - "which indicates that the corresponding `_arg` values hold a time period\n" - "in nanoseconds. The code also uses `scan_end_arg` which hold the number\n" - "of \"conversions\" within each \"scan\". The goal is to end up with the\n" - "scan period being less than or equal to the convert period multiplied by\n" - "the number of conversions per scan. It intends to do that by clamping\n" - "the `convert_arg` value to a maximum value of `UINT_MAX / scan_end_arg`\n" - "rounded down to a multiple of 1000 (`NSEC_PER_USEC`).\n" + "When console flow control is enabled (UPF_CONS_FLOW),\n" + "s3c24xx_serial_stop_tx() calls s3c24xx_serial_rx_enable() and\n" + "s3c24xx_serial_start_tx() calls s3c24xx_serial_rx_disable().\n" "\n" - "(The rounding from nanoseconds to microseconds is because the driver is\n" - "modelling a device that uses a 1 MHz clock for timing. This is partly\n" - "because that is a more typical timing base for real hardware devices\n" - "driven by comedi, and partly because the driver used to use `struct\n" - "timeval` internally.)\n" + "The serial core framework invokes the .stop_tx() and .start_tx()\n" + "callbacks with the port->lock spinlock already held. Furthermore, all\n" + "internal driver paths that invoke stop_tx (such as the DMA TX\n" + "completion handler s3c24xx_serial_tx_dma_complete() or the PIO TX IRQ\n" + "handler s3c24xx_serial_tx_irq()) also acquire port->lock prior to\n" + "calling it. (Note that s3c24xx_serial_start_tx() is only invoked by the\n" + "serial core).\n" "\n" - "The first bug is that the code checks if `scan_begin_arg == TRIG_TIMER`\n" - "when it should be checking if `scan_begin_src == TRIG_TIMER`. The\n" - "bugged check will always fail because if `scan_begin_src == TRIG_TIMER`,\n" - "then `scan_begin_arg` will be at least 1000 (`NSEC_PER_USEC`), otherwise\n" - "`scan_begin_src == TRIG_FOLLOW` and `scan_begin_arg` will be 0. (N.B\n" - "`TRIG_TIMER` is defined as `0x10`.) The second bug is that is rounding\n" - "the maximum value down to a multiple of 1000000000 (`NSEC_PER_SEC`)\n" - "instead of 1000 (`NSEC_PER_USEC`), however this bug is not reached due\n" - "to the first bug. This patch fixes both bugs.\n" + "However, s3c24xx_serial_rx_enable() and s3c24xx_serial_rx_disable()\n" + "unconditionally attempt to acquire port->lock again using\n" + "uart_port_lock_irqsave(). Since spinlocks are not recursive, this\n" + "causes a deadlock on the same CPU when console flow control is engaged.\n" + "\n" + "Remove the redundant lock acquisition from both rx helper functions.\n" "\n" - "Fixes: 783ddaebd397 (\"staging: comedi: comedi_test: support scan_begin_src == TRIG_FOLLOW\")\n" - "Fixes: 5afdcad2f818 (\"staging: comedi: comedi_test: limit maximum convert_arg\")\n" "Cc: stable <stable@kernel.org>\n" - "Signed-off-by: Ian Abbott <abbotti@mev.co.uk>\n" - "Link: https://patch.msgid.link/20260422144637.27692-1-abbotti@mev.co.uk\n" + "Fixes: b497549a035c (\"[ARM] S3C24XX: Split serial driver into core and per-cpu drivers\")\n" + "Reported-by: John Ogness <john.ogness@linutronix.de>\n" + "Closes: https://sashiko.dev/#/patchset/20260506121606.5805-1-john.ogness%40linutronix.de [1]\n" + "Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>\n" + "Link: https://patch.msgid.link/20260515-samsung-tty-flow-control-deadlock-v1-1-93255edbc9bc@linaro.org\n" "Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>\n" "Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>\n" "---\n" - " drivers/comedi/drivers/comedi_test.c | 4 ++--\n" - " 1 file changed, 2 insertions(+), 2 deletions(-)\n" + " drivers/tty/serial/samsung_tty.c | 8 --------\n" + " 1 file changed, 8 deletions(-)\n" "\n" - "--- a/drivers/comedi/drivers/comedi_test.c\n" - "+++ b/drivers/comedi/drivers/comedi_test.c\n" - "@@ -325,10 +325,10 @@ static int waveform_ai_cmdtest(struct co\n" - " \t\targ = min(arg,\n" - " \t\t\t rounddown(UINT_MAX, (unsigned int)NSEC_PER_USEC));\n" - " \t\targ = NSEC_PER_USEC * DIV_ROUND_CLOSEST(arg, NSEC_PER_USEC);\n" - "-\t\tif (cmd->scan_begin_arg == TRIG_TIMER) {\n" - "+\t\tif (cmd->scan_begin_src == TRIG_TIMER) {\n" - " \t\t\t/* limit convert_arg to keep scan_begin_arg in range */\n" - " \t\t\tlimit = UINT_MAX / cmd->scan_end_arg;\n" - "-\t\t\tlimit = rounddown(limit, (unsigned int)NSEC_PER_SEC);\n" - "+\t\t\tlimit = rounddown(limit, (unsigned int)NSEC_PER_USEC);\n" - " \t\t\targ = min(arg, limit);\n" - " \t\t}\n" - " \t\terr |= comedi_check_trigger_arg_is(&cmd->convert_arg, arg);" + "--- a/drivers/tty/serial/samsung_tty.c\n" + "+++ b/drivers/tty/serial/samsung_tty.c\n" + "@@ -243,12 +243,9 @@ static bool s3c24xx_serial_txempty_nofif\n" + " static void s3c24xx_serial_rx_enable(struct uart_port *port)\n" + " {\n" + " \tstruct s3c24xx_uart_port *ourport = to_ourport(port);\n" + "-\tunsigned long flags;\n" + " \tint count = 10000;\n" + " \tu32 ucon, ufcon;\n" + " \n" + "-\tuart_port_lock_irqsave(port, &flags);\n" + "-\n" + " \twhile (--count && !s3c24xx_serial_txempty_nofifo(port))\n" + " \t\tudelay(100);\n" + " \n" + "@@ -261,23 +258,18 @@ static void s3c24xx_serial_rx_enable(str\n" + " \twr_regl(port, S3C2410_UCON, ucon);\n" + " \n" + " \tourport->rx_enabled = 1;\n" + "-\tuart_port_unlock_irqrestore(port, flags);\n" + " }\n" + " \n" + " static void s3c24xx_serial_rx_disable(struct uart_port *port)\n" + " {\n" + " \tstruct s3c24xx_uart_port *ourport = to_ourport(port);\n" + "-\tunsigned long flags;\n" + " \tu32 ucon;\n" + " \n" + "-\tuart_port_lock_irqsave(port, &flags);\n" + "-\n" + " \tucon = rd_regl(port, S3C2410_UCON);\n" + " \tucon &= ~S3C2410_UCON_RXIRQMODE;\n" + " \twr_regl(port, S3C2410_UCON, ucon);\n" + " \n" + " \tourport->rx_enabled = 0;\n" + "-\tuart_port_unlock_irqrestore(port, flags);\n" + " }\n" + " \n" + static void s3c24xx_serial_stop_tx(struct uart_port *port) -9c3c1fc69772b04c0c6f402143277d8ce37fd91c308864ad0d722e338ddbfa42 +d8404e717da0138f83c108bfebe3cf560db46e85e973dde68f0642a34ba2dc69
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox