serial: sc16is7xx: fix invalid sc16is7xx_lines bitfield in case of probe error

commit 8a1060ce974919f2a79807527ad82ac39336eda2 upstream.

If an error occurs during probing, the sc16is7xx_lines bitfield may be left
in a state that doesn't represent the correct state of lines allocation.

For example, in a system with two SC16 devices, if an error occurs only
during probing of channel (port) B of the second device, sc16is7xx_lines
final state will be 00001011b instead of the expected 00000011b.

This is caused in part because of the "i--" in the for/loop located in
the out_ports: error path.

Fix this by checking the return value of uart_add_one_port() and set line
allocation bit only if this was successful. This allows the refactor of
the obfuscated for(i--...) loop in the error path, and properly call
uart_remove_one_port() only when needed, and properly unset line allocation
bits.

Also use same mechanism in remove() when calling uart_remove_one_port().

Fixes: c64349722d14 ("sc16is7xx: support multiple devices")
Cc:  <stable@vger.kernel.org>
Cc: Yury Norov <yury.norov@gmail.com>
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Link: https://lore.kernel.org/r/20231221231823.2327894-2-hugo@hugovil.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 826849d..06f9f33 100644 (file)
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -409,19 +409,6 @@ static void sc16is7xx_port_update(struct uart_port *port, u8 reg,
        regmap_update_bits(one->regmap, reg, mask, val);
 }
 
-static int sc16is7xx_alloc_line(void)
-{
-       int i;
-
-       BUILD_BUG_ON(SC16IS7XX_MAX_DEVS > BITS_PER_LONG);
-
-       for (i = 0; i < SC16IS7XX_MAX_DEVS; i++)
-               if (!test_and_set_bit(i, &sc16is7xx_lines))
-                       break;
-
-       return i;
-}
-
 static void sc16is7xx_power(struct uart_port *port, int on)
 {
        sc16is7xx_port_update(port, SC16IS7XX_IER_REG,
@@ -1532,6 +1519,13 @@ static int sc16is7xx_probe(struct device *dev,
                     SC16IS7XX_IOCONTROL_SRESET_BIT);
 
        for (i = 0; i < devtype->nr_uart; ++i) {
+               s->p[i].port.line = find_first_zero_bit(&sc16is7xx_lines,
+                                                       SC16IS7XX_MAX_DEVS);
+               if (s->p[i].port.line >= SC16IS7XX_MAX_DEVS) {
+                       ret = -ERANGE;
+                       goto out_ports;
+               }
+
                /* Initialize port data */
                s->p[i].port.dev        = dev;
                s->p[i].port.irq        = irq;
@@ -1551,14 +1545,8 @@ static int sc16is7xx_probe(struct device *dev,
                s->p[i].port.rs485_supported = sc16is7xx_rs485_supported;
                s->p[i].port.ops        = &sc16is7xx_ops;
                s->p[i].old_mctrl       = 0;
-               s->p[i].port.line       = sc16is7xx_alloc_line();
                s->p[i].regmap          = regmaps[i];
 
-               if (s->p[i].port.line >= SC16IS7XX_MAX_DEVS) {
-                       ret = -ENOMEM;
-                       goto out_ports;
-               }
-
                mutex_init(&s->p[i].efr_lock);
 
                ret = uart_get_rs485_mode(&s->p[i].port);
@@ -1576,8 +1564,13 @@ static int sc16is7xx_probe(struct device *dev,
                kthread_init_work(&s->p[i].tx_work, sc16is7xx_tx_proc);
                kthread_init_work(&s->p[i].reg_work, sc16is7xx_reg_proc);
                kthread_init_delayed_work(&s->p[i].ms_work, sc16is7xx_ms_proc);
+
                /* Register port */
-               uart_add_one_port(&sc16is7xx_uart, &s->p[i].port);
+               ret = uart_add_one_port(&sc16is7xx_uart, &s->p[i].port);
+               if (ret)
+                       goto out_ports;
+
+               set_bit(s->p[i].port.line, &sc16is7xx_lines);
 
                /* Enable EFR */
                sc16is7xx_port_write(&s->p[i].port, SC16IS7XX_LCR_REG,
@@ -1644,10 +1637,9 @@ static int sc16is7xx_probe(struct device *dev,
 #endif
 
 out_ports:
-       for (i--; i >= 0; i--) {
-               uart_remove_one_port(&sc16is7xx_uart, &s->p[i].port);
-               clear_bit(s->p[i].port.line, &sc16is7xx_lines);
-       }
+       for (i = 0; i < devtype->nr_uart; i++)
+               if (test_and_clear_bit(s->p[i].port.line, &sc16is7xx_lines))
+                       uart_remove_one_port(&sc16is7xx_uart, &s->p[i].port);
 
        kthread_stop(s->kworker_task);
 
@@ -1669,8 +1661,8 @@ static void sc16is7xx_remove(struct device *dev)
 
        for (i = 0; i < s->devtype->nr_uart; i++) {
                kthread_cancel_delayed_work_sync(&s->p[i].ms_work);
-               uart_remove_one_port(&sc16is7xx_uart, &s->p[i].port);
-               clear_bit(s->p[i].port.line, &sc16is7xx_lines);
+               if (test_and_clear_bit(s->p[i].port.line, &sc16is7xx_lines))
+                       uart_remove_one_port(&sc16is7xx_uart, &s->p[i].port);
                sc16is7xx_power(&s->p[i].port, 0);
        }