* [PATCH 1/3] serial: liteuart: fix compile testing
[not found] <20211115133745.11445-1-johan@kernel.org>
@ 2021-11-15 13:37 ` Johan Hovold
2021-11-15 21:15 ` Stafford Horne
2021-11-16 15:44 ` Andy Shevchenko
2021-11-15 13:37 ` [PATCH 2/3] serial: liteuart: fix use-after-free and memleak on unbind Johan Hovold
2021-11-15 13:37 ` [PATCH 3/3] serial: liteuart: fix minor-number leak on probe errors Johan Hovold
2 siblings, 2 replies; 9+ messages in thread
From: Johan Hovold @ 2021-11-15 13:37 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Ilia Sergachev, Karol Gugala, Mateusz Holenko, linux-serial,
linux-kernel, Johan Hovold, stable, Filip Kokosinski,
Stafford Horne
Allow the liteuart driver to be compile tested by fixing the broken
Kconfig dependencies.
Fixes: 1da81e5562fa ("drivers/tty/serial: add LiteUART driver")
Cc: stable@vger.kernel.org # 5.11
Cc: Filip Kokosinski <fkokosinski@antmicro.com>
Cc: Mateusz Holenko <mholenko@antmicro.com>
Cc: Stafford Horne <shorne@gmail.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/tty/serial/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 6ff94cfcd9db..67de892e0947 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1531,9 +1531,9 @@ config SERIAL_MILBEAUT_USIO_CONSOLE
config SERIAL_LITEUART
tristate "LiteUART serial port support"
+ depends on LITEX || COMPILE_TEST
depends on HAS_IOMEM
- depends on OF || COMPILE_TEST
- depends on LITEX
+ depends on OF
select SERIAL_CORE
help
This driver is for the FPGA-based LiteUART serial controller from LiteX
--
2.32.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/3] serial: liteuart: fix compile testing
2021-11-15 13:37 ` [PATCH 1/3] serial: liteuart: fix compile testing Johan Hovold
@ 2021-11-15 21:15 ` Stafford Horne
2021-11-16 15:44 ` Andy Shevchenko
1 sibling, 0 replies; 9+ messages in thread
From: Stafford Horne @ 2021-11-15 21:15 UTC (permalink / raw)
To: Johan Hovold
Cc: Greg Kroah-Hartman, Ilia Sergachev, Karol Gugala, Mateusz Holenko,
linux-serial, linux-kernel, stable, Filip Kokosinski
On Mon, Nov 15, 2021 at 02:37:43PM +0100, Johan Hovold wrote:
> Allow the liteuart driver to be compile tested by fixing the broken
> Kconfig dependencies.
>
> Fixes: 1da81e5562fa ("drivers/tty/serial: add LiteUART driver")
> Cc: stable@vger.kernel.org # 5.11
> Cc: Filip Kokosinski <fkokosinski@antmicro.com>
> Cc: Mateusz Holenko <mholenko@antmicro.com>
> Cc: Stafford Horne <shorne@gmail.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>
Reviewed-by: Stafford Horne <shorne@gmail.com>
> ---
> drivers/tty/serial/Kconfig | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 6ff94cfcd9db..67de892e0947 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1531,9 +1531,9 @@ config SERIAL_MILBEAUT_USIO_CONSOLE
>
> config SERIAL_LITEUART
> tristate "LiteUART serial port support"
> + depends on LITEX || COMPILE_TEST
> depends on HAS_IOMEM
> - depends on OF || COMPILE_TEST
> - depends on LITEX
> + depends on OF
> select SERIAL_CORE
> help
> This driver is for the FPGA-based LiteUART serial controller from LiteX
> --
> 2.32.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/3] serial: liteuart: fix compile testing
2021-11-15 13:37 ` [PATCH 1/3] serial: liteuart: fix compile testing Johan Hovold
2021-11-15 21:15 ` Stafford Horne
@ 2021-11-16 15:44 ` Andy Shevchenko
2021-11-17 9:06 ` Johan Hovold
1 sibling, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2021-11-16 15:44 UTC (permalink / raw)
To: Johan Hovold
Cc: Greg Kroah-Hartman, Ilia Sergachev, Karol Gugala, Mateusz Holenko,
open list:SERIAL DRIVERS, Linux Kernel Mailing List, Stable,
Filip Kokosinski, Stafford Horne
On Mon, Nov 15, 2021 at 3:44 PM Johan Hovold <johan@kernel.org> wrote:
>
> Allow the liteuart driver to be compile tested by fixing the broken
> Kconfig dependencies.
...
> config SERIAL_LITEUART
> tristate "LiteUART serial port support"
> + depends on LITEX || COMPILE_TEST
> depends on HAS_IOMEM
> - depends on OF || COMPILE_TEST
> - depends on LITEX
> + depends on OF
AFAICS this is optional and prevents compile testing in some cases.
> select SERIAL_CORE
> help
> This driver is for the FPGA-based LiteUART serial controller from LiteX
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] serial: liteuart: fix compile testing
2021-11-16 15:44 ` Andy Shevchenko
@ 2021-11-17 9:06 ` Johan Hovold
0 siblings, 0 replies; 9+ messages in thread
From: Johan Hovold @ 2021-11-17 9:06 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Greg Kroah-Hartman, Ilia Sergachev, Karol Gugala, Mateusz Holenko,
open list:SERIAL DRIVERS, Linux Kernel Mailing List, Stable,
Filip Kokosinski, Stafford Horne
On Tue, Nov 16, 2021 at 05:44:14PM +0200, Andy Shevchenko wrote:
> On Mon, Nov 15, 2021 at 3:44 PM Johan Hovold <johan@kernel.org> wrote:
> >
> > Allow the liteuart driver to be compile tested by fixing the broken
> > Kconfig dependencies.
>
> ...
>
> > config SERIAL_LITEUART
> > tristate "LiteUART serial port support"
> > + depends on LITEX || COMPILE_TEST
> > depends on HAS_IOMEM
> > - depends on OF || COMPILE_TEST
> > - depends on LITEX
>
> > + depends on OF
>
> AFAICS this is optional and prevents compile testing in some cases.
Yeah, you're right; that clause should stay. I'll send a v2. Thanks.
> > select SERIAL_CORE
> > help
> > This driver is for the FPGA-based LiteUART serial controller from LiteX
Johan
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] serial: liteuart: fix use-after-free and memleak on unbind
[not found] <20211115133745.11445-1-johan@kernel.org>
2021-11-15 13:37 ` [PATCH 1/3] serial: liteuart: fix compile testing Johan Hovold
@ 2021-11-15 13:37 ` Johan Hovold
2021-11-15 21:15 ` Stafford Horne
2021-11-15 13:37 ` [PATCH 3/3] serial: liteuart: fix minor-number leak on probe errors Johan Hovold
2 siblings, 1 reply; 9+ messages in thread
From: Johan Hovold @ 2021-11-15 13:37 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Ilia Sergachev, Karol Gugala, Mateusz Holenko, linux-serial,
linux-kernel, Johan Hovold, stable, Filip Kokosinski,
Stafford Horne
Deregister the port when unbinding the driver to prevent it from being
used after releasing the driver data and leaking memory allocated by
serial core.
Fixes: 1da81e5562fa ("drivers/tty/serial: add LiteUART driver")
Cc: stable@vger.kernel.org # 5.11
Cc: Filip Kokosinski <fkokosinski@antmicro.com>
Cc: Mateusz Holenko <mholenko@antmicro.com>
Cc: Stafford Horne <shorne@gmail.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/tty/serial/liteuart.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
index f075f4ff5fcf..da792d0df790 100644
--- a/drivers/tty/serial/liteuart.c
+++ b/drivers/tty/serial/liteuart.c
@@ -295,6 +295,7 @@ static int liteuart_remove(struct platform_device *pdev)
struct uart_port *port = platform_get_drvdata(pdev);
struct liteuart_port *uart = to_liteuart_port(port);
+ uart_remove_one_port(&liteuart_driver, port);
xa_erase(&liteuart_array, uart->id);
return 0;
--
2.32.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 2/3] serial: liteuart: fix use-after-free and memleak on unbind
2021-11-15 13:37 ` [PATCH 2/3] serial: liteuart: fix use-after-free and memleak on unbind Johan Hovold
@ 2021-11-15 21:15 ` Stafford Horne
2021-11-16 7:06 ` Johan Hovold
0 siblings, 1 reply; 9+ messages in thread
From: Stafford Horne @ 2021-11-15 21:15 UTC (permalink / raw)
To: Johan Hovold
Cc: Greg Kroah-Hartman, Ilia Sergachev, Karol Gugala, Mateusz Holenko,
linux-serial, linux-kernel, stable, Filip Kokosinski
On Mon, Nov 15, 2021 at 02:37:44PM +0100, Johan Hovold wrote:
> Deregister the port when unbinding the driver to prevent it from being
> used after releasing the driver data and leaking memory allocated by
> serial core.
This looks good to me. Just curious did you test this on a Litex SoC/FPGA?
> Fixes: 1da81e5562fa ("drivers/tty/serial: add LiteUART driver")
> Cc: stable@vger.kernel.org # 5.11
> Cc: Filip Kokosinski <fkokosinski@antmicro.com>
> Cc: Mateusz Holenko <mholenko@antmicro.com>
> Cc: Stafford Horne <shorne@gmail.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>
Reviewed-by: Stafford Horne <shorne@gmail.com>
> ---
> drivers/tty/serial/liteuart.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
> index f075f4ff5fcf..da792d0df790 100644
> --- a/drivers/tty/serial/liteuart.c
> +++ b/drivers/tty/serial/liteuart.c
> @@ -295,6 +295,7 @@ static int liteuart_remove(struct platform_device *pdev)
> struct uart_port *port = platform_get_drvdata(pdev);
> struct liteuart_port *uart = to_liteuart_port(port);
>
> + uart_remove_one_port(&liteuart_driver, port);
> xa_erase(&liteuart_array, uart->id);
>
> return 0;
> --
> 2.32.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 2/3] serial: liteuart: fix use-after-free and memleak on unbind
2021-11-15 21:15 ` Stafford Horne
@ 2021-11-16 7:06 ` Johan Hovold
0 siblings, 0 replies; 9+ messages in thread
From: Johan Hovold @ 2021-11-16 7:06 UTC (permalink / raw)
To: Stafford Horne
Cc: Greg Kroah-Hartman, Ilia Sergachev, Karol Gugala, Mateusz Holenko,
linux-serial, linux-kernel, stable, Filip Kokosinski
On Tue, Nov 16, 2021 at 06:15:05AM +0900, Stafford Horne wrote:
> On Mon, Nov 15, 2021 at 02:37:44PM +0100, Johan Hovold wrote:
> > Deregister the port when unbinding the driver to prevent it from being
> > used after releasing the driver data and leaking memory allocated by
> > serial core.
>
> This looks good to me. Just curious did you test this on a Litex
> SoC/FPGA?
No, this series has only been compile tested.
> > Fixes: 1da81e5562fa ("drivers/tty/serial: add LiteUART driver")
> > Cc: stable@vger.kernel.org # 5.11
> > Cc: Filip Kokosinski <fkokosinski@antmicro.com>
> > Cc: Mateusz Holenko <mholenko@antmicro.com>
> > Cc: Stafford Horne <shorne@gmail.com>
> > Signed-off-by: Johan Hovold <johan@kernel.org>
>
> Reviewed-by: Stafford Horne <shorne@gmail.com>
Thanks for reviewing.
Johan
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] serial: liteuart: fix minor-number leak on probe errors
[not found] <20211115133745.11445-1-johan@kernel.org>
2021-11-15 13:37 ` [PATCH 1/3] serial: liteuart: fix compile testing Johan Hovold
2021-11-15 13:37 ` [PATCH 2/3] serial: liteuart: fix use-after-free and memleak on unbind Johan Hovold
@ 2021-11-15 13:37 ` Johan Hovold
2021-11-15 21:16 ` Stafford Horne
2 siblings, 1 reply; 9+ messages in thread
From: Johan Hovold @ 2021-11-15 13:37 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Ilia Sergachev, Karol Gugala, Mateusz Holenko, linux-serial,
linux-kernel, Johan Hovold, stable, Filip Kokosinski,
Stafford Horne
Make sure to release the allocated minor number before returning on
probe errors.
Fixes: 1da81e5562fa ("drivers/tty/serial: add LiteUART driver")
Cc: stable@vger.kernel.org # 5.11
Cc: Filip Kokosinski <fkokosinski@antmicro.com>
Cc: Mateusz Holenko <mholenko@antmicro.com>
Cc: Stafford Horne <shorne@gmail.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/tty/serial/liteuart.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
index da792d0df790..2941659e5274 100644
--- a/drivers/tty/serial/liteuart.c
+++ b/drivers/tty/serial/liteuart.c
@@ -270,8 +270,10 @@ static int liteuart_probe(struct platform_device *pdev)
/* get membase */
port->membase = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
- if (IS_ERR(port->membase))
- return PTR_ERR(port->membase);
+ if (IS_ERR(port->membase)) {
+ ret = PTR_ERR(port->membase);
+ goto err_erase_id;
+ }
/* values not from device tree */
port->dev = &pdev->dev;
@@ -287,7 +289,16 @@ static int liteuart_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, port);
- return uart_add_one_port(&liteuart_driver, &uart->port);
+ ret = uart_add_one_port(&liteuart_driver, &uart->port);
+ if (ret)
+ goto err_erase_id;
+
+ return 0;
+
+err_erase_id:
+ xa_erase(&liteuart_array, uart->id);
+
+ return ret;
}
static int liteuart_remove(struct platform_device *pdev)
--
2.32.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] serial: liteuart: fix minor-number leak on probe errors
2021-11-15 13:37 ` [PATCH 3/3] serial: liteuart: fix minor-number leak on probe errors Johan Hovold
@ 2021-11-15 21:16 ` Stafford Horne
0 siblings, 0 replies; 9+ messages in thread
From: Stafford Horne @ 2021-11-15 21:16 UTC (permalink / raw)
To: Johan Hovold
Cc: Greg Kroah-Hartman, Ilia Sergachev, Karol Gugala, Mateusz Holenko,
linux-serial, linux-kernel, stable, Filip Kokosinski
On Mon, Nov 15, 2021 at 02:37:45PM +0100, Johan Hovold wrote:
> Make sure to release the allocated minor number before returning on
> probe errors.
>
> Fixes: 1da81e5562fa ("drivers/tty/serial: add LiteUART driver")
> Cc: stable@vger.kernel.org # 5.11
> Cc: Filip Kokosinski <fkokosinski@antmicro.com>
> Cc: Mateusz Holenko <mholenko@antmicro.com>
> Cc: Stafford Horne <shorne@gmail.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>
Reviewed-by: Stafford Horne <shorne@gmail.com>
> ---
> drivers/tty/serial/liteuart.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
> index da792d0df790..2941659e5274 100644
> --- a/drivers/tty/serial/liteuart.c
> +++ b/drivers/tty/serial/liteuart.c
> @@ -270,8 +270,10 @@ static int liteuart_probe(struct platform_device *pdev)
>
> /* get membase */
> port->membase = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
> - if (IS_ERR(port->membase))
> - return PTR_ERR(port->membase);
> + if (IS_ERR(port->membase)) {
> + ret = PTR_ERR(port->membase);
> + goto err_erase_id;
> + }
>
> /* values not from device tree */
> port->dev = &pdev->dev;
> @@ -287,7 +289,16 @@ static int liteuart_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, port);
>
> - return uart_add_one_port(&liteuart_driver, &uart->port);
> + ret = uart_add_one_port(&liteuart_driver, &uart->port);
> + if (ret)
> + goto err_erase_id;
> +
> + return 0;
> +
> +err_erase_id:
> + xa_erase(&liteuart_array, uart->id);
> +
> + return ret;
> }
>
> static int liteuart_remove(struct platform_device *pdev)
> --
> 2.32.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread