Re: [PATCH] serial: serial_core: Perform NULL checks for break_ctl ops

[Greg KH]

On Wed, Nov 20, 2019 at 11:18:53PM +0800, Jiangfeng Xiao wrote:
> Doing fuzz test on sbsa uart device, causes a kernel crash
> due to NULL pointer dereference:
> 
> ------------[ cut here ]------------
> Unable to handle kernel paging request at virtual address fffffffffffffffc
> pgd = ffffffe331723000
> [fffffffffffffffc] *pgd=0000002333595003, *pud=0000002333595003, *pmd=00000
> Internal error: Oops: 96000005 [#1] PREEMPT SMP
> Modules linked in: ping(O) jffs2 rtos_snapshot(O) pramdisk(O) hisi_sfc(O)
> Drv_Nandc_K(O) Drv_SysCtl_K(O) Drv_SysClk_K(O) bsp_reg(O) hns3(O)
> hns3_uio_enet(O) hclgevf(O) hclge(O) hnae3(O) mdio_factory(O)
> mdio_registry(O) mdio_dev(O) mdio(O) hns3_info(O) rtos_kbox_panic(O)
> uart_suspend(O) rsm(O) stp llc tunnel4 xt_tcpudp ipt_REJECT nf_reject_ipv4
> iptable_filter ip_tables x_tables sd_mod xhci_plat_hcd xhci_pci xhci_hcd
> usbmon usbhid usb_storage ohci_platform ohci_pci ohci_hcd hid_generic hid
> ehci_platform ehci_pci ehci_hcd vfat fat usbcore usb_common scsi_mod
> yaffs2multi(O) ext4 jbd2 ext2 mbcache ofpart i2c_dev i2c_core uio ubi nand
> nand_ecc nand_ids cfi_cmdset_0002 cfi_cmdset_0001 cfi_probe gen_probe
> cmdlinepart chipreg mtdblock mtd_blkdevs mtd nfsd auth_rpcgss oid_registry
> nfsv3 nfs nfs_acl lockd sunrpc grace autofs4
> CPU: 2 PID: 2385 Comm: tty_fuzz_test Tainted: G           O    4.4.193 #1
> task: ffffffe32b23f110 task.stack: ffffffe32bda4000
> PC is at uart_break_ctl+0x44/0x84
> LR is at uart_break_ctl+0x34/0x84
> pc : [<ffffff8393196098>] lr : [<ffffff8393196088>] pstate: 80000005
> sp : ffffffe32bda7cc0
> x29: ffffffe32bda7cc0 x28: ffffffe32b23f110
> x27: ffffff8393402000 x26: 0000000000000000
> x25: ffffffe32b233f40 x24: ffffffc07a8ec680
> x23: 0000000000005425 x22: 00000000ffffffff
> x21: ffffffe33ed73c98 x20: 0000000000000000
> x19: ffffffe33ed94168 x18: 0000000000000004
> x17: 0000007f92ae9d30 x16: ffffff8392fa6064
> x15: 0000000000000010 x14: 0000000000000000
> x13: 0000000000000000 x12: 0000000000000000
> x11: 0000000000000020 x10: 0000007ffdac1708
> x9 : 0000000000000078 x8 : 000000000000001d
> x7 : 0000000052a64887 x6 : ffffffe32bda7e08
> x5 : ffffffe32b23c000 x4 : 0000005fbc5b0000
> x3 : ffffff83938d5018 x2 : 0000000000000080
> x1 : ffffffe32b23c040 x0 : ffffff83934428f8
> virtual start addr offset is 38ac00000
> module base offset is 2cd4cf1000
> linear region base offset is : 0
> Process tty_fuzz_test (pid: 2385, stack limit = 0xffffffe32bda4000)
> Stack: (0xffffffe32bda7cc0 to 0xffffffe32bda8000)
> 7cc0: ffffffe32bda7cf0 ffffff8393177718 ffffffc07a8ec680 ffffff8393196054
> 7ce0: 000000001739f2e0 0000007ffdac1978 ffffffe32bda7d20 ffffff8393179a1c
> 7d00: 0000000000000000 ffffff8393c0a000 ffffffc07a8ec680 cb88537fdc8ba600
> 7d20: ffffffe32bda7df0 ffffff8392fa5a40 ffffff8393c0a000 0000000000005425
> 7d40: 0000007ffdac1978 ffffffe32b233f40 ffffff8393178dcc 0000000000000003
> 7d60: 000000000000011d 000000000000001d ffffffe32b23f110 000000000000029e
> 7d80: ffffffe34fe8d5d0 0000000000000000 ffffffe32bda7e14 cb88537fdc8ba600
> 7da0: ffffffe32bda7e30 ffffff8393042cfc ffffff8393c41720 ffffff8393c46410
> 7dc0: ffffff839304fa68 ffffffe32b233f40 0000000000005425 0000007ffdac1978
> 7de0: 000000000000011d cb88537fdc8ba600 ffffffe32bda7e70 ffffff8392fa60cc
> 7e00: 0000000000000000 ffffffe32b233f40 ffffffe32b233f40 0000000000000003
> 7e20: 0000000000005425 0000007ffdac1978 ffffffe32bda7e70 ffffff8392fa60b0
> 7e40: 0000000000000280 ffffffe32b233f40 ffffffe32b233f40 0000000000000003
> 7e60: 0000000000005425 cb88537fdc8ba600 0000000000000000 ffffff8392e02e78
> 7e80: 0000000000000280 0000005fbc5b0000 ffffffffffffffff 0000007f92ae9d3c
> 7ea0: 0000000060000000 0000000000000015 0000000000000003 0000000000005425
> 7ec0: 0000007ffdac1978 0000000000000000 00000000a54c910e 0000007f92b95014
> 7ee0: 0000007f92b95090 0000000052a64887 000000000000001d 0000000000000078
> 7f00: 0000007ffdac1708 0000000000000020 0000000000000000 0000000000000000
> 7f20: 0000000000000000 0000000000000010 000000556acf0090 0000007f92ae9d30
> 7f40: 0000000000000004 000000556acdef10 0000000000000000 000000556acdebd0
> 7f60: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> 7f80: 0000000000000000 0000000000000000 0000000000000000 0000007ffdac1840
> 7fa0: 000000556acdedcc 0000007ffdac1840 0000007f92ae9d3c 0000000060000000
> 7fc0: 0000000000000000 0000000000000000 0000000000000003 000000000000001d
> 7fe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> Call trace:
> Exception stack(0xffffffe32bda7ab0 to 0xffffffe32bda7bf0)
> 7aa0:                                   0000000000001000 0000007fffffffff
> 7ac0: ffffffe32bda7cc0 ffffff8393196098 0000000080000005 0000000000000025
> 7ae0: ffffffe32b233f40 ffffff83930d777c ffffffe32bda7b30 ffffff83930d777c
> 7b00: ffffffe32bda7be0 ffffff83938d5000 ffffffe32bda7be0 ffffffe32bda7c20
> 7b20: ffffffe32bda7b60 ffffff83930d777c ffffffe32bda7c10 ffffff83938d5000
> 7b40: ffffffe32bda7c10 ffffffe32bda7c50 ffffff8393c0a000 ffffffe32b23f110
> 7b60: ffffffe32bda7b70 ffffff8392e09df4 ffffffe32bda7bb0 cb88537fdc8ba600
> 7b80: ffffff83934428f8 ffffffe32b23c040 0000000000000080 ffffff83938d5018
> 7ba0: 0000005fbc5b0000 ffffffe32b23c000 ffffffe32bda7e08 0000000052a64887
> 7bc0: 000000000000001d 0000000000000078 0000007ffdac1708 0000000000000020
> 7be0: 0000000000000000 0000000000000000
> [<ffffff8393196098>] uart_break_ctl+0x44/0x84
> [<ffffff8393177718>] send_break+0xa0/0x114
> [<ffffff8393179a1c>] tty_ioctl+0xc50/0xe84
> [<ffffff8392fa5a40>] do_vfs_ioctl+0xc4/0x6e8
> [<ffffff8392fa60cc>] SyS_ioctl+0x68/0x9c
> [<ffffff8392e02e78>] __sys_trace_return+0x0/0x4
> Code: b9410ea0 34000160 f9408aa0 f9402814 (b85fc280)
> ---[ end trace 8606094f1960c5e0 ]---
> Kernel panic - not syncing: Fatal exception
> 
> Fix this problem by adding NULL checks prior to calling break_ctl ops.
> 
> Signed-off-by: Jiangfeng Xiao <xiaojiangfeng@xxxxxxxxxx>
> ---
>  drivers/tty/serial/serial_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index c4a414a..b0a6eb1 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -1111,7 +1111,7 @@ static int uart_break_ctl(struct tty_struct *tty, int break_state)
>  	if (!uport)
>  		goto out;
>  
> -	if (uport->type != PORT_UNKNOWN)
> +	if (uport->type != PORT_UNKNOWN && uport->ops->break_ctl)

What serial driver does not define break_ctl?

You are running with a bunch of "out-of-tree" drivers, perhaps one of
those needs to be fixed here instead?

thanks,

greg k-h