[PATCH v1] serial: sh-sci: fix memory region release in error path

zenghongling posted 1 patch 2 months, 1 week ago
There is a newer version of this series
drivers/tty/serial/sh-sci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH v1] serial: sh-sci: fix memory region release in error path
Posted by zenghongling 2 months, 1 week ago
From: Hongling Zeng <zenghongling@kylinos.cn>

The sci_request_port() function uses request_mem_region() to reserve
I/O memory, but in the error path when sci_remap_port() fails, it
incorrectly calls release_resource() instead of release_mem_region().

This mismatch can cause resource accounting issues. Fix it by using
the correct release function, consistent with sci_release_port().

Fixes: 2667bd6673eb ("serial: 8250_port: simplify serial8250_request_std_resource()")
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <error27@gmail.com>
Closes: https://lore.kernel.org/r/202604032356.SzEjYkBC-lkp@intel.com/
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

---
Change in v1:
  - Fix the commit message and change name
---
---
 drivers/tty/serial/sh-sci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index bd7486315338..9e619db27237 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -3024,7 +3024,7 @@ int sci_request_port(struct uart_port *port)
 
 	ret = sci_remap_port(port);
 	if (unlikely(ret != 0)) {
-		release_resource(res);
+		release_mem_region(port->mapbase, sport->reg_size);
 		return ret;
 	}
 
-- 
2.25.1
Re: [PATCH v1] serial: sh-sci: fix memory region release in error path
Posted by Geert Uytterhoeven 2 months ago
Hi Zeng,

On Mon, 13 Apr 2026 at 06:08, zenghongling <zenghongling@kylinos.cn> wrote:
> From: Hongling Zeng <zenghongling@kylinos.cn>
>
> The sci_request_port() function uses request_mem_region() to reserve
> I/O memory, but in the error path when sci_remap_port() fails, it
> incorrectly calls release_resource() instead of release_mem_region().
>
> This mismatch can cause resource accounting issues. Fix it by using
> the correct release function, consistent with sci_release_port().

Thanks for your patch!

> Fixes: 2667bd6673eb ("serial: 8250_port: simplify serial8250_request_std_resource()")

That commit does not touch the sh-sci driver.  Shouldn't that be
Fixes: e2651647080930a1 ("serial: sh-sci: Handle port memory region
reservations.")
instead?

"git log --follow" does a really bad job here, and doesn't even see the
offending commit, due to the actual change happening in parallel with
the move from drivers/serial to drivers/tty/serial.  Fortunately the
"git blame"-style output in the linked report below does show the
correct commit.

> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <error27@gmail.com>
> Closes: https://lore.kernel.org/r/202604032356.SzEjYkBC-lkp@intel.com/
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -3024,7 +3024,7 @@ int sci_request_port(struct uart_port *port)
>
>         ret = sci_remap_port(port);
>         if (unlikely(ret != 0)) {
> -               release_resource(res);
> +               release_mem_region(port->mapbase, sport->reg_size);
>                 return ret;
>         }
>

My first thought was "Isn't release_resource() just a shorthand in
case you already have a pointer to the resource?", but that indeed
seems to leak the resource, as it doesn't call free_resource()
(see __release_region()).

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Still, I'd like to see a second review opinion.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds