[PATCH] serial: 8250: null ptr deref in has_acpi_companion

[PATCH] serial: 8250: null ptr deref in has_acpi_companion

[Filip Jensen]

uart->port.dev is set in serial8250_register_8250_port if the port number
is under the initial CONFIG_SERIAL_8250_RUNTIME_UARTS. If unset, it will
depend upon up->port.dev being set by the particular serial driver. A
faulty driver might not set this value and this leads to null pointer
dereferenced later in has_acpi_companion and later at uart_add_one_port
call:

Oops: general protection fault, probably for non-canonical address
KASAN: null-ptr-deref in range
RIP: 0010:serial8250_register_8250_port+0xd34/0x2030
drivers/tty/serial/8250/8250_core.c

Reviewed-by: Jose Javier Rodriguez Barbarin <dev-josejavier.rodriguez@duagon.com>
Signed-off-by: Filip Jensen <dev-Felipe.Jensen@duagon.com>
---

For a case of a low level driver not setting this, cf.
Link: https://lore.kernel.org/linux-serial/20260305162815.41818-1-dev-Felipe.Jensen@duagon.com/

 drivers/tty/serial/8250/8250_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index d2e2c5dfef99..8ac3f4efa7fd 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -756,6 +756,8 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
 		if (ret)
 			goto err;
 	}
+	if (!uart->port.dev)
+		goto err;
 
 	if (up->port.flags & UPF_FIXED_TYPE)
 		uart->port.type = up->port.type;
-- 
2.34.1

Re: [PATCH] serial: 8250: null ptr deref in has_acpi_companion

[Jiri Slaby]

Hi,

1) the subject says:
[PATCH] serial: 8250: null ptr deref in has_acpi_companion

I read it as you are *adding* a null ptr dereference.

2) do not cut&paste literally the ./scripts/get_maintainer.pl output. I 
doubt Ingo and Thomas or others will help you with tty :P.

On 05. 03. 26, 17:33, Filip Jensen wrote:
> uart->port.dev is set in serial8250_register_8250_port if the port number
> is under the initial CONFIG_SERIAL_8250_RUNTIME_UARTS. If unset, it will
> depend upon up->port.dev being set by the particular serial driver. A
> faulty driver might not set this value and this leads to null pointer
> dereferenced later in has_acpi_companion and later at uart_add_one_port
> call:
> 
> Oops: general protection fault, probably for non-canonical address
> KASAN: null-ptr-deref in range
> RIP: 0010:serial8250_register_8250_port+0xd34/0x2030
> drivers/tty/serial/8250/8250_core.c
> 
> Reviewed-by: Jose Javier Rodriguez Barbarin <dev-josejavier.rodriguez@duagon.com>
> Signed-off-by: Filip Jensen <dev-Felipe.Jensen@duagon.com>
> ---
> 
> For a case of a low level driver not setting this, cf.
> Link: https://lore.kernel.org/linux-serial/20260305162815.41818-1-dev-Felipe.Jensen@duagon.com/
> 
>   drivers/tty/serial/8250/8250_core.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index d2e2c5dfef99..8ac3f4efa7fd 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -756,6 +756,8 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
>   		if (ret)
>   			goto err;
>   	}
> +	if (!uart->port.dev)
> +		goto err;

So ret actually can be unitialized, right?

thanks,
-- 
js
suse labs