Re: [PATCH] serial_core: fix ownership on allocated tty_driver

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

 



Hi Michel,

On 05/19/2016 07:35 AM, Michel Stam wrote:
> When removing a serial port driver (tested on 8250) while reading
> from one of the serial ports, the system will crash.
> This happens when the program closes the port.
> 
> Using the following test program:
> 	#include <unistd.h>
> 	#include <sys/types.h>
> 	#include <sys/stat.h>
> 	#include <fcntl.h>
> 
> 	int main(int argc, char **argv)
> 	{
> 	        int fd;
> 
> 	        fd = open( "/dev/ttyS0", O_RDONLY);
> 
> 	        sleep(10);
> 
> 	        close(fd);
> 	        return 0;
> 	}
> 
> If this program is running and the 8250 kernel driver is removed
> using 'rmmod', the system will panic with the following error:
> 	BUG: unable to handle kernel paging request at f2a82b3f
> 	IP: [<c11f9d0a>] strchr+0xa/0x20
> 	Oops: 0000 [#1] PREEMPT SMP
> 	CPU: 4 PID: 873 Comm: kworker/4:2 Tainted: P           O    4.4.11 #2
> 	Workqueue: events release_one_tty
> 	Call Trace:
> 	 [<c115548d>] __xlate_proc_name+0x4d/0xa0
> 	 [<c1155e88>] remove_proc_entry+0x28/0x150
> 	 [<c1069912>] ? put_prev_entity+0x22/0x950
> 	 [<c115836b>] proc_tty_unregister_driver+0x1b/0x30
> 	 [<c126faeb>] destruct_tty_driver+0x6b/0xc0
> 	 [<c126fb5a>] tty_driver_kref_put+0x1a/0x20
> 	 [<c126fd52>] release_one_tty+0x42/0x90
> 	 [<c105497e>] process_one_work+0xee/0x2b0
> 	 [<c1054c2d>] worker_thread+0xed/0x410
> 	 [<c1054b40>] ? process_one_work+0x2b0/0x2b0
> 	 [<c105953c>] kthread+0x9c/0xb0
> 	 [<c139ae98>] ? _raw_spin_unlock_irq+0x8/0x20
> 	 [<c139b389>] ret_from_kernel_thread+0x21/0x38
> 	 [<c10594a0>] ? kthread_create_on_node+0x120/0x120
> 
> The reason is that __xlate_proc_name() calls strchr() on a
> pointer to (struct tty_driver*)->name which is passed to it
> from proc_tty_unregister_driver() via remove_proc_entry().
> 
> The problem is caused by drivers (such as the 8250) having a
>  static struct uart_driver ...
> 
> which is removed from memory when rmmod is called. The
> .driver_name member of this structure is passed on to the
> struct tty_driver in drivers/tty/serial/serial_core:
> uart_register_driver()
> 
> The problem can be avoided by having a modules' reference
> counter incremented when devices start using the serial port.
> This functionality is present in the tty core, but not used
> because the .owner parameter is not assigned when the
> tty_driver struct is allocated.


Instead of making the uart driver module unloadable, the
plan is to properly proxy the uart driver and uart port.

New to 4.7 is my series that references the uart port
for in-progress operations, which prevents uart_remove_one_port()
from removing the port during actual use.

The next step is completely decouple uart_driver from tty_driver
(trivial) and refcount the uart_state/tty_port for last-use
destruction (non-trival).

Regards,
Peter Hurley


> We have been able to reproduce this problem on 3.10.34,
> 3.10.49 and 4.4.11.
> 
> Signed-off-by: Michel Stam <m.stam@xxxxxxxxx>
> Signed-off-by: Tjalling Hattink <t.hattink@xxxxxxxxx>
> ---
>  drivers/tty/serial/serial_core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index def5199..323e800 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -2413,6 +2413,7 @@ int uart_register_driver(struct uart_driver *drv)
>  
>  	drv->tty_driver = normal;
>  
> +	normal->owner		= drv->owner;
>  	normal->driver_name	= drv->driver_name;
>  	normal->name		= drv->dev_name;
>  	normal->major		= drv->major;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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