linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] serial: serial_core: Perform NULL checks for break_ctl ops
@ 2019-11-20 15:18 Jiangfeng Xiao
  2019-11-20 15:42 ` Greg KH
  0 siblings, 1 reply; 4+ messages in thread
From: Jiangfeng Xiao @ 2019-11-20 15:18 UTC (permalink / raw)
  To: gregkh, jslaby, xiaojiangfeng
  Cc: linux-serial, linux-kernel, leeyou.li, nixiaoming, zhangwen8

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@huawei.com>
---
 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)
 		uport->ops->break_ctl(uport, break_state);
 	ret = 0;
 out:
-- 
1.8.5.6


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] serial: serial_core: Perform NULL checks for break_ctl ops
  2019-11-20 15:18 [PATCH] serial: serial_core: Perform NULL checks for break_ctl ops Jiangfeng Xiao
@ 2019-11-20 15:42 ` Greg KH
  2019-11-21  2:14   ` Jiangfeng Xiao
  0 siblings, 1 reply; 4+ messages in thread
From: Greg KH @ 2019-11-20 15:42 UTC (permalink / raw)
  To: Jiangfeng Xiao
  Cc: jslaby, linux-serial, linux-kernel, leeyou.li, nixiaoming, zhangwen8

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@huawei.com>
> ---
>  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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] serial: serial_core: Perform NULL checks for break_ctl ops
  2019-11-20 15:42 ` Greg KH
@ 2019-11-21  2:14   ` Jiangfeng Xiao
  2019-11-21  7:11     ` Greg KH
  0 siblings, 1 reply; 4+ messages in thread
From: Jiangfeng Xiao @ 2019-11-21  2:14 UTC (permalink / raw)
  To: Greg KH
  Cc: jslaby, linux-serial, linux-kernel, leeyou.li, nixiaoming, zhangwen8

On 2019/11/20 23:42, Greg KH wrote:
> 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
>> 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
>> [<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@huawei.com>
>> ---
>>  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?

I'm using arm_sbsa_uart_platform_driver, and sbsa_uart_pops does not define break_ctl.
I did a troubleshooting on the latest mainline 5.4-rc8 kernel.
There are 7 uart_ops without defining break_ctl:
./drivers/tty/serial/men_z135_uart.c:774:static const struct uart_ops men_z135_ops = {
./drivers/tty/serial/amba-pl011.c:2173:static const struct uart_ops sbsa_uart_pops = {
./drivers/tty/serial/owl-uart.c:464:static const struct uart_ops owl_uart_ops = {
./drivers/tty/serial/meson_uart.c:430:static const struct uart_ops meson_uart_ops = {
./drivers/tty/serial/qcom_geni_serial.c:1212:static const struct uart_ops qcom_geni_console_pops = {
./drivers/tty/serial/qcom_geni_serial.c:1232:static const struct uart_ops qcom_geni_uart_pops = {
./drivers/tty/serial/rda-uart.c:559:static const struct uart_ops rda_uart_ops = {

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

I carefully confirmed it again, the kernel crash was caused by arm_sbsa_uart_platform_driver which is "in-tree".

There are two ways to fix this problem, one is to add a null pointer check,
and the other is to define break_ctl for each uart_ops.
I am a newbie on serial_core, so I don't know which way is more reasonable.

Based on the principle of minimal change, I chose to add a null pointer check.

Thank you for your review.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] serial: serial_core: Perform NULL checks for break_ctl ops
  2019-11-21  2:14   ` Jiangfeng Xiao
@ 2019-11-21  7:11     ` Greg KH
  0 siblings, 0 replies; 4+ messages in thread
From: Greg KH @ 2019-11-21  7:11 UTC (permalink / raw)
  To: Jiangfeng Xiao
  Cc: jslaby, linux-serial, linux-kernel, leeyou.li, nixiaoming, zhangwen8

On Thu, Nov 21, 2019 at 10:14:02AM +0800, Jiangfeng Xiao wrote:
> On 2019/11/20 23:42, Greg KH wrote:
> > 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
> >> 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
> >> [<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@huawei.com>
> >> ---
> >>  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?
> 
> I'm using arm_sbsa_uart_platform_driver, and sbsa_uart_pops does not define break_ctl.
> I did a troubleshooting on the latest mainline 5.4-rc8 kernel.
> There are 7 uart_ops without defining break_ctl:
> ./drivers/tty/serial/men_z135_uart.c:774:static const struct uart_ops men_z135_ops = {
> ./drivers/tty/serial/amba-pl011.c:2173:static const struct uart_ops sbsa_uart_pops = {
> ./drivers/tty/serial/owl-uart.c:464:static const struct uart_ops owl_uart_ops = {
> ./drivers/tty/serial/meson_uart.c:430:static const struct uart_ops meson_uart_ops = {
> ./drivers/tty/serial/qcom_geni_serial.c:1212:static const struct uart_ops qcom_geni_console_pops = {
> ./drivers/tty/serial/qcom_geni_serial.c:1232:static const struct uart_ops qcom_geni_uart_pops = {
> ./drivers/tty/serial/rda-uart.c:559:static const struct uart_ops rda_uart_ops = {
> 
> > You are running with a bunch of "out-of-tree" drivers, perhaps one of
> > those needs to be fixed here instead?
> 
> I carefully confirmed it again, the kernel crash was caused by arm_sbsa_uart_platform_driver which is "in-tree".
> 
> There are two ways to fix this problem, one is to add a null pointer check,
> and the other is to define break_ctl for each uart_ops.
> I am a newbie on serial_core, so I don't know which way is more reasonable.
> 
> Based on the principle of minimal change, I chose to add a null pointer check.

Yeah, that makes sense to me as well, I'll go queue this up.  Thanks for
the detailed information.

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-11-21  7:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-20 15:18 [PATCH] serial: serial_core: Perform NULL checks for break_ctl ops Jiangfeng Xiao
2019-11-20 15:42 ` Greg KH
2019-11-21  2:14   ` Jiangfeng Xiao
2019-11-21  7:11     ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).