Skip to content

Commit 801410b

Browse files
pccgregkh
authored andcommitted
serial: Lock console when calling into driver before registration
During the handoff from earlycon to the real console driver, we have two separate drivers operating on the same device concurrently. In the case of the 8250 driver these concurrent accesses cause problems due to the driver's use of banked registers, controlled by LCR.DLAB. It is possible for the setup(), config_port(), pm() and set_mctrl() callbacks to set DLAB, which can cause the earlycon code that intends to access TX to instead access DLL, leading to missed output and corruption on the serial line due to unintended modifications to the baud rate. In particular, for setup() we have: univ8250_console_setup() -> serial8250_console_setup() -> uart_set_options() -> serial8250_set_termios() -> serial8250_do_set_termios() -> serial8250_do_set_divisor() For config_port() we have: serial8250_config_port() -> autoconfig() For pm() we have: serial8250_pm() -> serial8250_do_pm() -> serial8250_set_sleep() For set_mctrl() we have (for some devices): serial8250_set_mctrl() -> omap8250_set_mctrl() -> __omap8250_set_mctrl() To avoid such problems, let's make it so that the console is locked during pre-registration calls to these callbacks, which will prevent the earlycon driver from running concurrently. Remove the partial solution to this problem in the 8250 driver that locked the console only during autoconfig_irq(), as this would result in a deadlock with the new approach. The console continues to be locked during autoconfig_irq() because it can only be called through uart_configure_port(). Although this patch introduces more locking than strictly necessary (and in particular it also locks during the call to rs485_config() which is not affected by this issue as far as I can tell), it follows the principle that it is the responsibility of the generic console code to manage the earlycon handoff by ensuring that earlycon and real console driver code cannot run concurrently, and not the individual drivers. Signed-off-by: Peter Collingbourne <pcc@google.com> Reviewed-by: John Ogness <john.ogness@linutronix.de> Link: https://linux-review.googlesource.com/id/I7cf8124dcebf8618e6b2ee543fa5b25532de55d8 Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20240304214350.501253-1-pcc@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 26e8349 commit 801410b

3 files changed

Lines changed: 30 additions & 9 deletions

File tree

drivers/tty/serial/8250/8250_port.c

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1325,9 +1325,6 @@ static void autoconfig_irq(struct uart_8250_port *up)
13251325
inb_p(ICP);
13261326
}
13271327

1328-
if (uart_console(port))
1329-
console_lock();
1330-
13311328
/* forget possible initially masked and pending IRQ */
13321329
probe_irq_off(probe_irq_on());
13331330
save_mcr = serial8250_in_MCR(up);
@@ -1367,9 +1364,6 @@ static void autoconfig_irq(struct uart_8250_port *up)
13671364
if (port->flags & UPF_FOURPORT)
13681365
outb_p(save_ICP, ICP);
13691366

1370-
if (uart_console(port))
1371-
console_unlock();
1372-
13731367
port->irq = (irq > 0) ? irq : 0;
13741368
}
13751369

drivers/tty/serial/serial_core.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2608,14 +2608,23 @@ uart_configure_port(struct uart_driver *drv, struct uart_state *state,
26082608
port->type = PORT_UNKNOWN;
26092609
flags |= UART_CONFIG_TYPE;
26102610
}
2611+
/* Synchronize with possible boot console. */
2612+
if (uart_console(port))
2613+
console_lock();
26112614
port->ops->config_port(port, flags);
2615+
if (uart_console(port))
2616+
console_unlock();
26122617
}
26132618

26142619
if (port->type != PORT_UNKNOWN) {
26152620
unsigned long flags;
26162621

26172622
uart_report_port(drv, port);
26182623

2624+
/* Synchronize with possible boot console. */
2625+
if (uart_console(port))
2626+
console_lock();
2627+
26192628
/* Power up port for set_mctrl() */
26202629
uart_change_pm(state, UART_PM_STATE_ON);
26212630

@@ -2632,6 +2641,9 @@ uart_configure_port(struct uart_driver *drv, struct uart_state *state,
26322641

26332642
uart_rs485_config(port);
26342643

2644+
if (uart_console(port))
2645+
console_unlock();
2646+
26352647
/*
26362648
* If this driver supports console, and it hasn't been
26372649
* successfully registered yet, try to re-register it.

kernel/printk/printk.c

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3263,6 +3263,21 @@ static int __init keep_bootcon_setup(char *str)
32633263

32643264
early_param("keep_bootcon", keep_bootcon_setup);
32653265

3266+
static int console_call_setup(struct console *newcon, char *options)
3267+
{
3268+
int err;
3269+
3270+
if (!newcon->setup)
3271+
return 0;
3272+
3273+
/* Synchronize with possible boot console. */
3274+
console_lock();
3275+
err = newcon->setup(newcon, options);
3276+
console_unlock();
3277+
3278+
return err;
3279+
}
3280+
32663281
/*
32673282
* This is called by register_console() to try to match
32683283
* the newly registered console with any of the ones selected
@@ -3298,8 +3313,8 @@ static int try_enable_preferred_console(struct console *newcon,
32983313
if (_braille_register_console(newcon, c))
32993314
return 0;
33003315

3301-
if (newcon->setup &&
3302-
(err = newcon->setup(newcon, c->options)) != 0)
3316+
err = console_call_setup(newcon, c->options);
3317+
if (err)
33033318
return err;
33043319
}
33053320
newcon->flags |= CON_ENABLED;
@@ -3325,7 +3340,7 @@ static void try_enable_default_console(struct console *newcon)
33253340
if (newcon->index < 0)
33263341
newcon->index = 0;
33273342

3328-
if (newcon->setup && newcon->setup(newcon, NULL) != 0)
3343+
if (console_call_setup(newcon, NULL) != 0)
33293344
return;
33303345

33313346
newcon->flags |= CON_ENABLED;

0 commit comments

Comments
 (0)