From ce684552a266cb1c7cc2f7e623f38567adec6653 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Sun, 12 Jul 2020 20:10:12 +0900 Subject: vt: Reject zero-sized screen buffer size. syzbot is reporting general protection fault in do_con_write() [1] caused by vc->vc_screenbuf == ZERO_SIZE_PTR caused by vc->vc_screenbuf_size == 0 caused by vc->vc_cols == vc->vc_rows == vc->vc_size_row == 0 caused by fb_set_var() from ioctl(FBIOPUT_VSCREENINFO) on /dev/fb0 , for gotoxy(vc, 0, 0) from reset_terminal() from vc_init() from vc_allocate() from con_install() from tty_init_dev() from tty_open() on such console causes vc->vc_pos == 0x10000000e due to ((unsigned long) ZERO_SIZE_PTR) + -1U * 0 + (-1U << 1). I don't think that a console with 0 column or 0 row makes sense. And it seems that vc_do_resize() does not intend to allow resizing a console to 0 column or 0 row due to new_cols = (cols ? cols : vc->vc_cols); new_rows = (lines ? lines : vc->vc_rows); exception. Theoretically, cols and rows can be any range as long as 0 < cols * rows * 2 <= KMALLOC_MAX_SIZE is satisfied (e.g. cols == 1048576 && rows == 2 is possible) because of vc->vc_size_row = vc->vc_cols << 1; vc->vc_screenbuf_size = vc->vc_rows * vc->vc_size_row; in visual_init() and kzalloc(vc->vc_screenbuf_size) in vc_allocate(). Since we can detect cols == 0 or rows == 0 via screenbuf_size = 0 in visual_init(), we can reject kzalloc(0). Then, vc_allocate() will return an error, and con_write() will not be called on a console with 0 column or 0 row. We need to make sure that integer overflow in visual_init() won't happen. Since vc_do_resize() restricts cols <= 32767 and rows <= 32767, applying 1 <= cols <= 32767 and 1 <= rows <= 32767 restrictions to vc_allocate() will be practically fine. This patch does not touch con_init(), for returning -EINVAL there does not help when we are not returning -ENOMEM. [1] https://syzkaller.appspot.com/bug?extid=017265e8553724e514e8 Reported-and-tested-by: syzbot Signed-off-by: Tetsuo Handa Cc: stable Link: https://lore.kernel.org/r/20200712111013.11881-1-penguin-kernel@I-love.SAKURA.ne.jp Signed-off-by: Greg Kroah-Hartman --- drivers/tty/vt/vt.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c index 48a8199f7845..42d8c67a481f 100644 --- a/drivers/tty/vt/vt.c +++ b/drivers/tty/vt/vt.c @@ -1092,10 +1092,19 @@ static const struct tty_port_operations vc_port_ops = { .destruct = vc_port_destruct, }; +/* + * Change # of rows and columns (0 means unchanged/the size of fg_console) + * [this is to be used together with some user program + * like resize that changes the hardware videomode] + */ +#define VC_MAXCOL (32767) +#define VC_MAXROW (32767) + int vc_allocate(unsigned int currcons) /* return 0 on success */ { struct vt_notifier_param param; struct vc_data *vc; + int err; WARN_CONSOLE_UNLOCKED(); @@ -1125,6 +1134,11 @@ int vc_allocate(unsigned int currcons) /* return 0 on success */ if (!*vc->vc_uni_pagedir_loc) con_set_default_unimap(vc); + err = -EINVAL; + if (vc->vc_cols > VC_MAXCOL || vc->vc_rows > VC_MAXROW || + vc->vc_screenbuf_size > KMALLOC_MAX_SIZE || !vc->vc_screenbuf_size) + goto err_free; + err = -ENOMEM; vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_KERNEL); if (!vc->vc_screenbuf) goto err_free; @@ -1143,7 +1157,7 @@ err_free: visual_deinit(vc); kfree(vc); vc_cons[currcons].d = NULL; - return -ENOMEM; + return err; } static inline int resize_screen(struct vc_data *vc, int width, int height, @@ -1158,14 +1172,6 @@ static inline int resize_screen(struct vc_data *vc, int width, int height, return err; } -/* - * Change # of rows and columns (0 means unchanged/the size of fg_console) - * [this is to be used together with some user program - * like resize that changes the hardware videomode] - */ -#define VC_RESIZE_MAXCOL (32767) -#define VC_RESIZE_MAXROW (32767) - /** * vc_do_resize - resizing method for the tty * @tty: tty being resized @@ -1201,7 +1207,7 @@ static int vc_do_resize(struct tty_struct *tty, struct vc_data *vc, user = vc->vc_resize_user; vc->vc_resize_user = 0; - if (cols > VC_RESIZE_MAXCOL || lines > VC_RESIZE_MAXROW) + if (cols > VC_MAXCOL || lines > VC_MAXROW) return -EINVAL; new_cols = (cols ? cols : vc->vc_cols); @@ -1212,7 +1218,7 @@ static int vc_do_resize(struct tty_struct *tty, struct vc_data *vc, if (new_cols == vc->vc_cols && new_rows == vc->vc_rows) return 0; - if (new_screen_size > KMALLOC_MAX_SIZE) + if (new_screen_size > KMALLOC_MAX_SIZE || !new_screen_size) return -EINVAL; newscreen = kzalloc(new_screen_size, GFP_USER); if (!newscreen) @@ -3393,6 +3399,7 @@ static int __init con_init(void) INIT_WORK(&vc_cons[currcons].SAK_work, vc_SAK); tty_port_init(&vc->port); visual_init(vc, currcons, 1); + /* Assuming vc->vc_{cols,rows,screenbuf_size} are sane here. */ vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_NOWAIT); vc_init(vc, vc->vc_rows, vc->vc_cols, currcons || !vc->vc_sw->con_save_screen); -- cgit v1.2.3 From 22a82fa7d6c3e16d56a036b1fa697a39b954adf0 Mon Sep 17 00:00:00 2001 From: Helmut Grohne Date: Mon, 13 Jul 2020 09:32:28 +0200 Subject: tty: xilinx_uartps: Really fix id assignment The problems started with the revert (18cc7ac8a28e28). The cdns_uart_console.index is statically assigned -1. When the port is registered, Linux assigns consecutive numbers to it. It turned out that when using ttyPS1 as console, the index is not updated as we are reusing the same cdns_uart_console instance for multiple ports. When registering ttyPS0, it gets updated from -1 to 0, but when registering ttyPS1, it already is 0 and not updated. That led to 2ae11c46d5fdc4. It assigns the index prior to registering the uart_driver once. Unfortunately, that ended up breaking the situation where the probe order does not match the id order. When using the same device tree for both uboot and linux, it is important that the serial0 alias points to the console. So some boards reverse those aliases. This was reported by Jan Kiszka. The proposed fix was reverting the index assignment and going back to the previous iteration. However such a reversed assignement (serial0 -> uart1, serial1 -> uart0) was already partially broken by the revert (18cc7ac8a28e28). While the ttyPS device works, the kmsg connection is already broken and kernel messages go missing. Reverting the id assignment does not fix this. >From the xilinx_uartps driver pov (after reverting the refactoring commits), there can be only one console. This manifests in static variables console_pprt and cdns_uart_console. These variables are not properly linked and can go out of sync. The cdns_uart_console.index is important for uart_add_one_port. We call that function for each port - one of which hopefully is the console. If it isn't, the CON_ENABLED flag is not set and console_port is cleared. The next cdns_uart_probe call then tries to register the next port using that same cdns_uart_console. It is important that console_port and cdns_uart_console (and its index in particular) stay in sync. The index assignment implemented by Shubhrajyoti Datta is correct in principle. It just may have to happen a second time if the first cdns_uart_probe call didn't encounter the console device. And we shouldn't change the index once the console uart is registered. Reported-by: Shubhrajyoti Datta Reported-by: Jan Kiszka Link: https://lore.kernel.org/linux-serial/f4092727-d8f5-5f91-2c9f-76643aace993@siemens.com/ Fixes: 18cc7ac8a28e28 ("Revert "serial: uartps: Register own uart console and driver structures"") Fixes: 2ae11c46d5fdc4 ("tty: xilinx_uartps: Fix missing id assignment to the console") Fixes: 76ed2e10579671 ("Revert "tty: xilinx_uartps: Fix missing id assignment to the console"") Signed-off-by: Helmut Grohne Cc: stable Link: https://lore.kernel.org/r/20200713073227.GA3805@laureti-dev Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/xilinx_uartps.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c index 672cfa075e28..2833f1418d6d 100644 --- a/drivers/tty/serial/xilinx_uartps.c +++ b/drivers/tty/serial/xilinx_uartps.c @@ -1580,8 +1580,10 @@ static int cdns_uart_probe(struct platform_device *pdev) * If register_console() don't assign value, then console_port pointer * is cleanup. */ - if (!console_port) + if (!console_port) { + cdns_uart_console.index = id; console_port = port; + } #endif rc = uart_add_one_port(&cdns_uart_uart_driver, port); @@ -1594,8 +1596,10 @@ static int cdns_uart_probe(struct platform_device *pdev) #ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE /* This is not port which is used for console that's why clean it up */ if (console_port == port && - !(cdns_uart_uart_driver.cons->flags & CON_ENABLED)) + !(cdns_uart_uart_driver.cons->flags & CON_ENABLED)) { console_port = NULL; + cdns_uart_console.index = -1; + } #endif cdns_uart_data->cts_override = of_property_read_bool(pdev->dev.of_node, -- cgit v1.2.3 From b374c562ee7ab3f3a1daf959c01868bae761571c Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Fri, 10 Jul 2020 15:59:46 +0200 Subject: serial: tegra: fix CREAD handling for PIO Commit 33ae787b74fc ("serial: tegra: add support to ignore read") added support for dropping input in case CREAD isn't set, but for PIO the ignore_status_mask wasn't checked until after the character had been put in the receive buffer. Note that the NULL tty-port test is bogus and will be removed by a follow-on patch. Fixes: 33ae787b74fc ("serial: tegra: add support to ignore read") Cc: stable # 5.4 Cc: Shardar Shariff Md Cc: Krishna Yarlagadda Signed-off-by: Johan Hovold Acked-by: Thierry Reding Link: https://lore.kernel.org/r/20200710135947.2737-2-johan@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/serial-tegra.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c index 8de8bac9c6c7..b3bbee6b6702 100644 --- a/drivers/tty/serial/serial-tegra.c +++ b/drivers/tty/serial/serial-tegra.c @@ -653,11 +653,14 @@ static void tegra_uart_handle_rx_pio(struct tegra_uart_port *tup, ch = (unsigned char) tegra_uart_read(tup, UART_RX); tup->uport.icount.rx++; - if (!uart_handle_sysrq_char(&tup->uport, ch) && tty) - tty_insert_flip_char(tty, ch, flag); + if (uart_handle_sysrq_char(&tup->uport, ch)) + continue; if (tup->uport.ignore_status_mask & UART_LSR_DR) continue; + + if (tty) + tty_insert_flip_char(tty, ch, flag); } while (1); } -- cgit v1.2.3 From 707631ce639651e51bfed9e56326cde86f9e97b8 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Fri, 10 Jul 2020 15:59:47 +0200 Subject: serial: tegra: drop bogus NULL tty-port checks The struct tty_port is part of the uart state and will never be NULL in the receive helpers. Drop the bogus NULL checks and rename the pointer-variables "port" to differentiate them from struct tty_struct pointers (which can be NULL). Fixes: 962963e4ee23 ("serial: tegra: Switch to using struct tty_port") Signed-off-by: Johan Hovold Acked-by: Thierry Reding Link: https://lore.kernel.org/r/20200710135947.2737-3-johan@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/serial-tegra.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c index b3bbee6b6702..04d1b0807e66 100644 --- a/drivers/tty/serial/serial-tegra.c +++ b/drivers/tty/serial/serial-tegra.c @@ -635,7 +635,7 @@ static void tegra_uart_handle_tx_pio(struct tegra_uart_port *tup) } static void tegra_uart_handle_rx_pio(struct tegra_uart_port *tup, - struct tty_port *tty) + struct tty_port *port) { do { char flag = TTY_NORMAL; @@ -659,13 +659,12 @@ static void tegra_uart_handle_rx_pio(struct tegra_uart_port *tup, if (tup->uport.ignore_status_mask & UART_LSR_DR) continue; - if (tty) - tty_insert_flip_char(tty, ch, flag); + tty_insert_flip_char(port, ch, flag); } while (1); } static void tegra_uart_copy_rx_to_tty(struct tegra_uart_port *tup, - struct tty_port *tty, + struct tty_port *port, unsigned int count) { int copied; @@ -675,17 +674,13 @@ static void tegra_uart_copy_rx_to_tty(struct tegra_uart_port *tup, return; tup->uport.icount.rx += count; - if (!tty) { - dev_err(tup->uport.dev, "No tty port\n"); - return; - } if (tup->uport.ignore_status_mask & UART_LSR_DR) return; dma_sync_single_for_cpu(tup->uport.dev, tup->rx_dma_buf_phys, count, DMA_FROM_DEVICE); - copied = tty_insert_flip_string(tty, + copied = tty_insert_flip_string(port, ((unsigned char *)(tup->rx_dma_buf_virt)), count); if (copied != count) { WARN_ON(1); -- cgit v1.2.3 From f4c23a140d80ef5e6d3d1f8f57007649014b60fa Mon Sep 17 00:00:00 2001 From: Yang Yingliang Date: Tue, 21 Jul 2020 14:38:52 +0000 Subject: serial: 8250: fix null-ptr-deref in serial8250_start_tx() I got null-ptr-deref in serial8250_start_tx(): [ 78.114630] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000 [ 78.123778] Mem abort info: [ 78.126560] ESR = 0x86000007 [ 78.129603] EC = 0x21: IABT (current EL), IL = 32 bits [ 78.134891] SET = 0, FnV = 0 [ 78.137933] EA = 0, S1PTW = 0 [ 78.141064] user pgtable: 64k pages, 48-bit VAs, pgdp=00000027d41a8600 [ 78.147562] [0000000000000000] pgd=00000027893f0003, p4d=00000027893f0003, pud=00000027893f0003, pmd=00000027c9a20003, pte=0000000000000000 [ 78.160029] Internal error: Oops: 86000007 [#1] SMP [ 78.164886] Modules linked in: sunrpc vfat fat aes_ce_blk crypto_simd cryptd aes_ce_cipher crct10dif_ce ghash_ce sha2_ce sha256_arm64 sha1_ce ses enclosure sg sbsa_gwdt ipmi_ssif spi_dw_mmio sch_fq_codel vhost_net tun vhost vhost_iotlb tap ip_tables ext4 mbcache jbd2 ahci hisi_sas_v3_hw libahci hisi_sas_main libsas hns3 scsi_transport_sas hclge libata megaraid_sas ipmi_si hnae3 ipmi_devintf ipmi_msghandler br_netfilter bridge stp llc nvme nvme_core xt_sctp sctp libcrc32c dm_mod nbd [ 78.207383] CPU: 11 PID: 23258 Comm: null-ptr Not tainted 5.8.0-rc6+ #48 [ 78.214056] Hardware name: Huawei TaiShan 2280 V2/BC82AMDC, BIOS 2280-V2 CS V3.B210.01 03/12/2020 [ 78.222888] pstate: 80400089 (Nzcv daIf +PAN -UAO BTYPE=--) [ 78.228435] pc : 0x0 [ 78.230618] lr : serial8250_start_tx+0x160/0x260 [ 78.235215] sp : ffff800062eefb80 [ 78.238517] x29: ffff800062eefb80 x28: 0000000000000fff [ 78.243807] x27: ffff800062eefd80 x26: ffff202fd83b3000 [ 78.249098] x25: ffff800062eefd80 x24: ffff202fd83b3000 [ 78.254388] x23: ffff002fc5e50be8 x22: 0000000000000002 [ 78.259679] x21: 0000000000000001 x20: 0000000000000000 [ 78.264969] x19: ffffa688827eecc8 x18: 0000000000000000 [ 78.270259] x17: 0000000000000000 x16: 0000000000000000 [ 78.275550] x15: ffffa68881bc67a8 x14: 00000000000002e6 [ 78.280841] x13: ffffa68881bc67a8 x12: 000000000000c539 [ 78.286131] x11: d37a6f4de9bd37a7 x10: ffffa68881cccff0 [ 78.291421] x9 : ffffa68881bc6000 x8 : ffffa688819daa88 [ 78.296711] x7 : ffffa688822a0f20 x6 : ffffa688819e0000 [ 78.302002] x5 : ffff800062eef9d0 x4 : ffffa68881e707a8 [ 78.307292] x3 : 0000000000000000 x2 : 0000000000000002 [ 78.312582] x1 : 0000000000000001 x0 : ffffa688827eecc8 [ 78.317873] Call trace: [ 78.320312] 0x0 [ 78.322147] __uart_start.isra.9+0x64/0x78 [ 78.326229] uart_start+0xb8/0x1c8 [ 78.329620] uart_flush_chars+0x24/0x30 [ 78.333442] n_tty_receive_buf_common+0x7b0/0xc30 [ 78.338128] n_tty_receive_buf+0x44/0x2c8 [ 78.342122] tty_ioctl+0x348/0x11f8 [ 78.345599] ksys_ioctl+0xd8/0xf8 [ 78.348903] __arm64_sys_ioctl+0x2c/0xc8 [ 78.352812] el0_svc_common.constprop.2+0x88/0x1b0 [ 78.357583] do_el0_svc+0x44/0xd0 [ 78.360887] el0_sync_handler+0x14c/0x1d0 [ 78.364880] el0_sync+0x140/0x180 [ 78.368185] Code: bad PC value SERIAL_PORT_DFNS is not defined on each arch, if it's not defined, serial8250_set_defaults() won't be called in serial8250_isa_init_ports(), so the p->serial_in pointer won't be initialized, and it leads a null-ptr-deref. Fix this problem by calling serial8250_set_defaults() after init uart port. Signed-off-by: Yang Yingliang Cc: stable Link: https://lore.kernel.org/r/20200721143852.4058352-1-yangyingliang@huawei.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index fc118f649887..cae61d1ebec5 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -524,6 +524,7 @@ static void __init serial8250_isa_init_ports(void) */ up->mcr_mask = ~ALPHA_KLUDGE_MCR; up->mcr_force = ALPHA_KLUDGE_MCR; + serial8250_set_defaults(up); } /* chain base port ops to support Remote Supervisor Adapter */ @@ -547,7 +548,6 @@ static void __init serial8250_isa_init_ports(void) port->membase = old_serial_port[i].iomem_base; port->iotype = old_serial_port[i].io_type; port->regshift = old_serial_port[i].iomem_reg_shift; - serial8250_set_defaults(up); port->irqflags |= irqflag; if (serial8250_isa_config != NULL) -- cgit v1.2.3 From 551e553f0d4ab623e2a6f424ab5834f9c7b5229c Mon Sep 17 00:00:00 2001 From: Serge Semin Date: Tue, 14 Jul 2020 15:41:12 +0300 Subject: serial: 8250_mtk: Fix high-speed baud rates clamping Commit 7b668c064ec3 ("serial: 8250: Fix max baud limit in generic 8250 port") fixed limits of a baud rate setting for a generic 8250 port. In other words since that commit the baud rate has been permitted to be within [uartclk / 16 / UART_DIV_MAX; uartclk / 16], which is absolutely normal for a standard 8250 UART port. But there are custom 8250 ports, which provide extended baud rate limits. In particular the Mediatek 8250 port can work with baud rates up to "uartclk" speed. Normally that and any other peculiarity is supposed to be handled in a custom set_termios() callback implemented in the vendor-specific 8250-port glue-driver. Currently that is how it's done for the most of the vendor-specific 8250 ports, but for some reason for Mediatek a solution has been spread out to both the glue-driver and to the generic 8250-port code. Due to that a bug has been introduced, which permitted the extended baud rate limit for all even for standard 8250-ports. The bug has been fixed by the commit 7b668c064ec3 ("serial: 8250: Fix max baud limit in generic 8250 port") by narrowing the baud rates limit back down to the normal bounds. Unfortunately by doing so we also broke the Mediatek-specific extended bauds feature. A fix of the problem described above is twofold. First since we can't get back the extended baud rate limits feature to the generic set_termios() function and that method supports only a standard baud rates range, the requested baud rate must be locally stored before calling it and then restored back to the new termios structure after the generic set_termios() finished its magic business. By doing so we still use the serial8250_do_set_termios() method to set the LCR/MCR/FCR/etc. registers, while the extended baud rate setting procedure will be performed later in the custom Mediatek-specific set_termios() callback. Second since a true baud rate is now fully calculated in the custom set_termios() method we need to locally update the port timeout by calling the uart_update_timeout() function. After the fixes described above are implemented in the 8250_mtk.c driver, the Mediatek 8250-port should get back to normally working with extended baud rates. Link: https://lore.kernel.org/linux-serial/20200701211337.3027448-1-danielwinkler@google.com Fixes: 7b668c064ec3 ("serial: 8250: Fix max baud limit in generic 8250 port") Reported-by: Daniel Winkler Signed-off-by: Serge Semin Cc: stable Tested-by: Claire Chang Link: https://lore.kernel.org/r/20200714124113.20918-1-Sergey.Semin@baikalelectronics.ru Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_mtk.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c index f839380c2f4c..98b8a3e30733 100644 --- a/drivers/tty/serial/8250/8250_mtk.c +++ b/drivers/tty/serial/8250/8250_mtk.c @@ -306,8 +306,21 @@ mtk8250_set_termios(struct uart_port *port, struct ktermios *termios, } #endif + /* + * Store the requested baud rate before calling the generic 8250 + * set_termios method. Standard 8250 port expects bauds to be + * no higher than (uartclk / 16) so the baud will be clamped if it + * gets out of that bound. Mediatek 8250 port supports speed + * higher than that, therefore we'll get original baud rate back + * after calling the generic set_termios method and recalculate + * the speed later in this method. + */ + baud = tty_termios_baud_rate(termios); + serial8250_do_set_termios(port, termios, old); + tty_termios_encode_baud_rate(termios, baud, baud); + /* * Mediatek UARTs use an extra highspeed register (MTK_UART_HIGHS) * @@ -339,6 +352,11 @@ mtk8250_set_termios(struct uart_port *port, struct ktermios *termios, */ spin_lock_irqsave(&port->lock, flags); + /* + * Update the per-port timeout. + */ + uart_update_timeout(port, termios->c_cflag, baud); + /* set DLAB we have cval saved in up->lcr from the call to the core */ serial_port_out(port, UART_LCR, up->lcr | UART_LCR_DLAB); serial_dl_write(up, quot); -- cgit v1.2.3 From 033724d6864245a11f8e04c066002e6ad22b3fd0 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Wed, 15 Jul 2020 10:51:02 +0900 Subject: fbdev: Detect integer underflow at "struct fbcon_ops"->clear_margins. syzbot is reporting general protection fault in bitfill_aligned() [1] caused by integer underflow in bit_clear_margins(). The cause of this problem is when and how do_vc_resize() updates vc->vc_{cols,rows}. If vc_do_resize() fails (e.g. kzalloc() fails) when var.xres or var.yres is going to shrink, vc->vc_{cols,rows} will not be updated. This allows bit_clear_margins() to see info->var.xres < (vc->vc_cols * cw) or info->var.yres < (vc->vc_rows * ch). Unexpectedly large rw or bh will try to overrun the __iomem region and causes general protection fault. Also, vc_resize(vc, 0, 0) does not set vc->vc_{cols,rows} = 0 due to new_cols = (cols ? cols : vc->vc_cols); new_rows = (lines ? lines : vc->vc_rows); exception. Since cols and lines are calculated as cols = FBCON_SWAP(ops->rotate, info->var.xres, info->var.yres); rows = FBCON_SWAP(ops->rotate, info->var.yres, info->var.xres); cols /= vc->vc_font.width; rows /= vc->vc_font.height; vc_resize(vc, cols, rows); in fbcon_modechanged(), var.xres < vc->vc_font.width makes cols = 0 and var.yres < vc->vc_font.height makes rows = 0. This means that const int fd = open("/dev/fb0", O_ACCMODE); struct fb_var_screeninfo var = { }; ioctl(fd, FBIOGET_VSCREENINFO, &var); var.xres = var.yres = 1; ioctl(fd, FBIOPUT_VSCREENINFO, &var); easily reproduces integer underflow bug explained above. Of course, callers of vc_resize() are not handling vc_do_resize() failure is bad. But we can't avoid vc_resize(vc, 0, 0) which returns 0. Therefore, as a band-aid workaround, this patch checks integer underflow in "struct fbcon_ops"->clear_margins call, assuming that vc->vc_cols * vc->vc_font.width and vc->vc_rows * vc->vc_font.heigh do not cause integer overflow. [1] https://syzkaller.appspot.com/bug?id=a565882df74fa76f10d3a6fec4be31098dbb37c6 Reported-and-tested-by: syzbot Signed-off-by: Tetsuo Handa Acked-by: Daniel Vetter Cc: stable Link: https://lore.kernel.org/r/20200715015102.3814-1-penguin-kernel@I-love.SAKURA.ne.jp Signed-off-by: Greg Kroah-Hartman --- drivers/video/fbdev/core/bitblit.c | 4 ++-- drivers/video/fbdev/core/fbcon_ccw.c | 4 ++-- drivers/video/fbdev/core/fbcon_cw.c | 4 ++-- drivers/video/fbdev/core/fbcon_ud.c | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/video/fbdev/core/bitblit.c b/drivers/video/fbdev/core/bitblit.c index ca935c09a261..35ebeeccde4d 100644 --- a/drivers/video/fbdev/core/bitblit.c +++ b/drivers/video/fbdev/core/bitblit.c @@ -216,7 +216,7 @@ static void bit_clear_margins(struct vc_data *vc, struct fb_info *info, region.color = color; region.rop = ROP_COPY; - if (rw && !bottom_only) { + if ((int) rw > 0 && !bottom_only) { region.dx = info->var.xoffset + rs; region.dy = 0; region.width = rw; @@ -224,7 +224,7 @@ static void bit_clear_margins(struct vc_data *vc, struct fb_info *info, info->fbops->fb_fillrect(info, ®ion); } - if (bh) { + if ((int) bh > 0) { region.dx = info->var.xoffset; region.dy = info->var.yoffset + bs; region.width = rs; diff --git a/drivers/video/fbdev/core/fbcon_ccw.c b/drivers/video/fbdev/core/fbcon_ccw.c index dfa9a8aa4509..78f3a5621478 100644 --- a/drivers/video/fbdev/core/fbcon_ccw.c +++ b/drivers/video/fbdev/core/fbcon_ccw.c @@ -201,7 +201,7 @@ static void ccw_clear_margins(struct vc_data *vc, struct fb_info *info, region.color = color; region.rop = ROP_COPY; - if (rw && !bottom_only) { + if ((int) rw > 0 && !bottom_only) { region.dx = 0; region.dy = info->var.yoffset; region.height = rw; @@ -209,7 +209,7 @@ static void ccw_clear_margins(struct vc_data *vc, struct fb_info *info, info->fbops->fb_fillrect(info, ®ion); } - if (bh) { + if ((int) bh > 0) { region.dx = info->var.xoffset + bs; region.dy = 0; region.height = info->var.yres_virtual; diff --git a/drivers/video/fbdev/core/fbcon_cw.c b/drivers/video/fbdev/core/fbcon_cw.c index ce08251bfd38..fd098ff17574 100644 --- a/drivers/video/fbdev/core/fbcon_cw.c +++ b/drivers/video/fbdev/core/fbcon_cw.c @@ -184,7 +184,7 @@ static void cw_clear_margins(struct vc_data *vc, struct fb_info *info, region.color = color; region.rop = ROP_COPY; - if (rw && !bottom_only) { + if ((int) rw > 0 && !bottom_only) { region.dx = 0; region.dy = info->var.yoffset + rs; region.height = rw; @@ -192,7 +192,7 @@ static void cw_clear_margins(struct vc_data *vc, struct fb_info *info, info->fbops->fb_fillrect(info, ®ion); } - if (bh) { + if ((int) bh > 0) { region.dx = info->var.xoffset; region.dy = info->var.yoffset; region.height = info->var.yres; diff --git a/drivers/video/fbdev/core/fbcon_ud.c b/drivers/video/fbdev/core/fbcon_ud.c index 1936afc78fec..e165a3fad29a 100644 --- a/drivers/video/fbdev/core/fbcon_ud.c +++ b/drivers/video/fbdev/core/fbcon_ud.c @@ -231,7 +231,7 @@ static void ud_clear_margins(struct vc_data *vc, struct fb_info *info, region.color = color; region.rop = ROP_COPY; - if (rw && !bottom_only) { + if ((int) rw > 0 && !bottom_only) { region.dy = 0; region.dx = info->var.xoffset; region.width = rw; @@ -239,7 +239,7 @@ static void ud_clear_margins(struct vc_data *vc, struct fb_info *info, info->fbops->fb_fillrect(info, ®ion); } - if (bh) { + if ((int) bh > 0) { region.dy = info->var.yoffset; region.dx = info->var.xoffset; region.height = bh; -- cgit v1.2.3 From 5fdbe136ae19ab751daaa4d08d9a42f3e30d17f9 Mon Sep 17 00:00:00 2001 From: Matthew Howell Date: Wed, 22 Jul 2020 16:11:24 -0400 Subject: serial: exar: Fix GPIO configuration for Sealevel cards based on XR17V35X Sealevel XR17V35X based devices are inoperable on kernel versions 4.11 and above due to a change in the GPIO preconfiguration introduced in commit 7dea8165f1d. This patch fixes this by preconfiguring the GPIO on Sealevel cards to the value (0x00) used prior to commit 7dea8165f1d With GPIOs preconfigured as per commit 7dea8165f1d all ports on Sealevel XR17V35X based devices become stuck in high impedance mode, regardless of dip-switch or software configuration. This causes the device to become effectively unusable. This patch (in various forms) has been distributed to our customers and no issues related to it have been reported. Fixes: 7dea8165f1d6 ("serial: exar: Preconfigure xr17v35x MPIOs as output") Signed-off-by: Matthew Howell Link: https://lore.kernel.org/r/alpine.DEB.2.21.2007221605270.13247@tstest-VirtualBox Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_exar.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c index ddb6aeb76dc5..04b9af7ed941 100644 --- a/drivers/tty/serial/8250/8250_exar.c +++ b/drivers/tty/serial/8250/8250_exar.c @@ -326,7 +326,17 @@ static void setup_gpio(struct pci_dev *pcidev, u8 __iomem *p) * devices will export them as GPIOs, so we pre-configure them safely * as inputs. */ - u8 dir = pcidev->vendor == PCI_VENDOR_ID_EXAR ? 0xff : 0x00; + + u8 dir = 0x00; + + if ((pcidev->vendor == PCI_VENDOR_ID_EXAR) && + (pcidev->subsystem_vendor != PCI_VENDOR_ID_SEALEVEL)) { + // Configure GPIO as inputs for Commtech adapters + dir = 0xff; + } else { + // Configure GPIO as outputs for SeaLevel adapters + dir = 0x00; + } writeb(0x00, p + UART_EXAR_MPIOINT_7_0); writeb(0x00, p + UART_EXAR_MPIOLVL_7_0); -- cgit v1.2.3