Re: [PATCH] serial: core: Fix race between freeing and writing to transmit buffer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10. 10. 18, 14:45, Ole Bjørn Midtbø  wrote:
> The state->xmit.buf can be freed, and set to NULL, by uart_shutdown while
> uart_put_char and uart_write is waiting to acquire the uart_port_lock
> causing a NULL pointer dereference.
> 
> Only observed through uart_write, but similar issue could happen if
> uart_put_char was called. The issue was triggered on an older kernel,
> version 4.4, but is present in current code as well.
> 
> Crash report:
> Unable to handle kernel NULL pointer dereference at virtual address 00000196
> ...
> Workqueue: events_unbound flush_to_ldisc
> task: ffffffc1eb1cdb00 ti: ffffffc1eac6c000 task.ti: ffffffc1eac6c000
> PC is at __memcpy+0xa0/0x180
> LR is at uart_write+0x158/0x240
> ...
> Call trace:
> [<ffffffc0004104a0>] __memcpy+0xa0/0x180
> [<ffffffc00057f1c0>] do_output_char+0xa8/0x1d8
> [<ffffffc00057f4bc>] __process_echoes+0x1cc/0x280
> [<ffffffc00057f66c>] commit_echoes+0x6c/0xb0
> [<ffffffc000581818>] n_tty_receive_char_special+0x7b8/0xa98
> [<ffffffc000582388>] n_tty_receive_buf_common+0x520/0xa38
> [<ffffffc0005828e0>] n_tty_receive_buf2+0x40/0x50
> [<ffffffc000585ae0>] flush_to_ldisc+0xd0/0x168
> [<ffffffc0000c1784>] process_one_work+0x2b4/0x4d8
> [<ffffffc0000c1c70>] worker_thread+0x2c8/0x4d8
> [<ffffffc0000c8944>] kthread+0x104/0x118
> [<ffffffc000084fa0>] ret_from_fork+0x10/0x30
> ---[ end trace 8fd3b56af7a64339 ]---
> 
> Signed-off-by: Ole Bjørn Midtbø <omidtbo@xxxxxxxxx>
> ---
>  drivers/tty/serial/serial_core.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 80bb56facfb6..39a016ffdd17 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -544,7 +544,7 @@ static int uart_put_char(struct tty_struct *tty, unsigned char c)
>  		return 0;

Correct, the port lock protects the circular buffer. But this 'return 0'
above belongs to 'if (!circ->buf)'. So the if should be removed (or
moved inside the lock).

>  	port = uart_port_lock(state, flags);
> -	if (port && uart_circ_chars_free(circ) != 0) {
> +	if (port && circ->buf && uart_circ_chars_free(circ) != 0) {
>  		circ->buf[circ->head] = c;
>  		circ->head = (circ->head + 1) & (UART_XMIT_SIZE - 1);
>  		ret = 1;
> @@ -581,6 +581,11 @@ static int uart_write(struct tty_struct *tty,
>  		return 0;
>  
>  	port = uart_port_lock(state, flags);
> +	if (!circ->buf) {
> +		uart_port_unlock(port, flags);
> +		return 0;
> +	}

The same here.

>  	while (port) {
>  		c = CIRC_SPACE_TO_END(circ->head, circ->tail, UART_XMIT_SIZE);
>  		if (count < c)
> 

thanks,
-- 
js
suse labs



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux