serial: 8250: Prevent kernel crash with nr_uarts=0

PATCH In review Jan Kiszka 2 participants 5 messages May 4, 2015 – May 5, 2015 TTY layer and serial drivers

Share & tools

Jan Kiszka

author

2015-05-04 16:01 UTC #0 lore raw

PATCH 1/1 serial: 8250: Prevent kernel crash with nr_uarts=0

When nr_uarts was set to 0 (via config or 8250_core.nr_uarts), we crash early on x86 because serial8250_isa_init_ports dereferences base_ops which remains NULL. In fact, there is nothing to do for that function if there are no uarts.

Signed-off-by Jan Kiszka <[email protected]>

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

drivers/tty/serial/8250/8250_core.c +1 -1

index 4506e40..e1363a40 100644
@@ -3256,7 +3256,7 @@ static void __init serial8250_isa_init_ports(void)
 	static int first = 1;
 	int i, irqflag = 0;
 
-	if (!first)
+	if (!first || nr_uarts == 0)
 		return;
 	first = 0;

Signature · 1 line

2.1.4

Peter Hurley

2015-05-04 17:02 UTC #1 lore raw

Hi Jan,

Quoted 4 lines · Jan Kiszka

When nr_uarts was set to 0 (via config or 8250_core.nr_uarts), we crash early on x86 because serial8250_isa_init_ports dereferences base_ops which remains NULL. In fact, there is nothing to do for that function if there are no uarts.

Thanks for finding this.

So nr_uarts == 0 effectively disables the 8250 driver. Is there any reason not to simply abort the driver init instead?

Regards,
Peter Hurley

Quoted 19 lines

Signed-off-by Jan Kiszka <[email protected]>

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

drivers/tty/serial/8250/8250_core.c +1 -1

index 4506e40..e1363a40 100644
@@ -3256,7 +3256,7 @@ static void __init serial8250_isa_init_ports(void)
 	static int first = 1;
 	int i, irqflag = 0;
 
-	if (!first)
+	if (!first || nr_uarts == 0)
 		return;
 	first = 0;

Jan Kiszka

author

2015-05-04 17:45 UTC #2 lore raw

Quoted 8 lines · Peter Hurley

Hi Jan,
Quoted 4 lines · Jan Kiszka
When nr_uarts was set to 0 (via config or 8250_core.nr_uarts), we crash early on x86 because serial8250_isa_init_ports dereferences base_ops which remains NULL. In fact, there is nothing to do for that function if there are no uarts.

Quoted 4 lines

Thanks for finding this.
So nr_uarts == 0 effectively disables the 8250 driver. Is there any reason not to simply abort the driver init instead?

I'm not very deep into this code, just stumbled over this while trying some, well, unusual configurations.

If you prefer to handle this differently, I can recode it, of course.

Jan

Signature · 2 lines

Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Peter Hurley

2015-05-04 18:52 UTC #3 lore raw

Quoted 9 lines · Jan Kiszka

Quoted 8 lines · Peter Hurley
Hi Jan,
Quoted 4 lines · Jan Kiszka
When nr_uarts was set to 0 (via config or 8250_core.nr_uarts), we crash early on x86 because serial8250_isa_init_ports dereferences base_ops which remains NULL. In fact, there is nothing to do for that function if there are no uarts.

Quoted 7 lines

Quoted 4 lines
Thanks for finding this.
So nr_uarts == 0 effectively disables the 8250 driver. Is there any reason not to simply abort the driver init instead?
I'm not very deep into this code, just stumbled over this while trying some, well, unusual configurations.

Ok; I wasn't sure if this was related to some weird setup that needed the 8250 driver loaded but in-op.

Quoted 1 line

If you prefer to handle this differently, I can recode it, of course.

Yes, please. We should bail out of any initialization if nr_uarts == 0.

That would be:
1. univ8250_console_init()
The return value is ignored but the console should not be registered.

2. early_serial_setup()

	if (nr_uarts == 0)
		return -ENODEV;

3. serial8250_init()

	if (nr_uarts == 0)
		return -ENODEV;

Regards,
Peter Hurley

Jan Kiszka

author

2015-05-05 06:26 UTC #4 lore raw

PATCH 1/1 serial: 8250: Do nothing if nr_uarts=0

Based on suggestions by Peter Hurley.

Signed-off-by Jan Kiszka <[email protected]>

---

This supersedes "serial: 8250: Prevent kernel crash with nr_uarts=0"

 drivers/tty/serial/8250/8250_core.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

drivers/tty/serial/8250/8250_core.c +7 -1

index 4506e40..dfcb14f 100644
@@ -3548,6 +3548,9 @@ static struct console univ8250_console = {
 
 static int __init univ8250_console_init(void)
 {
+	if (nr_uarts == 0)
+		return -ENODEV;
+
 	serial8250_isa_init_ports();
 	register_console(&univ8250_console);
 	return 0;

@@ -3578,7 +3581,7 @@ int __init early_serial_setup(struct uart_port *port)
 {
 	struct uart_port *p;
 
-	if (port->line >= ARRAY_SIZE(serial8250_ports))
+	if (port->line >= ARRAY_SIZE(serial8250_ports) || nr_uarts == 0)
 		return -ENODEV;
 
 	serial8250_isa_init_ports();

@@ -3945,6 +3948,9 @@ static int __init serial8250_init(void)
 {
 	int ret;
 
+	if (nr_uarts == 0)
+		return -ENODEV;
+
 	serial8250_isa_init_ports();
 
 	printk(KERN_INFO "Serial: 8250/16550 driver, "

Signature · 1 line

2.1.4