From: Hugo Villeneuve hvilleneuve@dimonoff.com
If a problem with a device occurs during probing, and we subsequently try to remove the driver, we get the following error:
$ rmmod sc16is7xx [ 62.783166] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000040 [ 62.994226] Call trace: [ 62.996672] serial_core_unregister_port+0x58/0x2b0 [ 63.001558] serial_ctrl_unregister_port+0x18/0x30 [ 63.006354] uart_remove_one_port+0x18/0x30 [ 63.010542] sc16is7xx_spi_remove+0xc0/0x190 [sc16is7xx] Segmentation fault
Tested on a custom board with two SC16IS742 ICs, and by simulating a fault when probing channel (port) B of the second device.
The reason is that uart_remove_one_port() has already been called during probe in the out_ports: error path, and is called again in sc16is7xx_remove().
Fix the problem by clearing the device drvdata in probe error path to indicate that all resources have been deallocated. And only deallocate resources in sc16is7xx_remove() if device drvdata is non-null.
Fixes: dfeae619d781 ("serial: sc16is7xx") Cc: stable@vger.kernel.org Signed-off-by: Hugo Villeneuve hvilleneuve@dimonoff.com --- drivers/tty/serial/sc16is7xx.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c index e40e4a99277e..b585663c1e6e 100644 --- a/drivers/tty/serial/sc16is7xx.c +++ b/drivers/tty/serial/sc16is7xx.c @@ -1663,6 +1663,8 @@ static int sc16is7xx_probe(struct device *dev, out_clk: clk_disable_unprepare(s->clk);
+ dev_set_drvdata(dev, NULL); + return ret; }
@@ -1671,6 +1673,9 @@ static void sc16is7xx_remove(struct device *dev) struct sc16is7xx_port *s = dev_get_drvdata(dev); int i;
+ if (!s) + return; + #ifdef CONFIG_GPIOLIB if (s->gpio_valid_mask) gpiochip_remove(&s->gpio);
On Tue, Dec 19, 2023 at 12:18:45PM -0500, Hugo Villeneuve wrote:
From: Hugo Villeneuve hvilleneuve@dimonoff.com
If a problem with a device occurs during probing, and we subsequently try to remove the driver, we get the following error:
$ rmmod sc16is7xx [ 62.783166] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000040 [ 62.994226] Call trace: [ 62.996672] serial_core_unregister_port+0x58/0x2b0 [ 63.001558] serial_ctrl_unregister_port+0x18/0x30 [ 63.006354] uart_remove_one_port+0x18/0x30 [ 63.010542] sc16is7xx_spi_remove+0xc0/0x190 [sc16is7xx] Segmentation fault
Tested on a custom board with two SC16IS742 ICs, and by simulating a fault when probing channel (port) B of the second device.
The reason is that uart_remove_one_port() has already been called during probe in the out_ports: error path, and is called again in sc16is7xx_remove().
Fix the problem by clearing the device drvdata in probe error path to indicate that all resources have been deallocated. And only deallocate resources in sc16is7xx_remove() if device drvdata is non-null.
...
- dev_set_drvdata(dev, NULL);
I believe this is wrong approach to fix the issue as this one is prone to be cleaned up in the future as we don't do this call explicitly for the past ~15 years.
On Wed, Dec 20, 2023 at 05:34:29PM +0200, Andy Shevchenko wrote:
On Tue, Dec 19, 2023 at 12:18:45PM -0500, Hugo Villeneuve wrote:
From: Hugo Villeneuve hvilleneuve@dimonoff.com
...
- dev_set_drvdata(dev, NULL);
I believe this is wrong approach to fix the issue as this one is prone to be cleaned up in the future as we don't do this call explicitly for the past ~15 years.
On top of that the ->remove() is not the only uart_remove_one_port() call. It has a lot of other stuff to go with.
It seems that ->remove() doesn't check the bit in &sc16is7xx_lines, that might be the proper fix for the issue you have.
linux-stable-mirror@lists.linaro.org