* BUG: unable to handle kernel NULL pointer dereference in mem16_serial_out @ 2019-12-09 19:35 syzbot 2019-12-10 1:38 ` syzbot 2021-04-26 16:14 ` [PATCH] serial: 8250: fix NULL pointer dereference in serial8250_do_startup() Vegard Nossum 0 siblings, 2 replies; 9+ messages in thread From: syzbot @ 2019-12-09 19:35 UTC (permalink / raw) To: andriy.shevchenko, asierra, ext-kimmo.rautkoski, gregkh, jslaby, kai.heng.feng, linux-kernel, linux-serial, mika.westerberg, paulburton, sr, syzkaller-bugs, yegorslists Hello, syzbot found the following crash on: HEAD commit: e42617b8 Linux 5.5-rc1 git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=1157cd41e00000 kernel config: https://syzkaller.appspot.com/x/.config?x=3754e2c78c1adb82 dashboard link: https://syzkaller.appspot.com/bug?extid=92f32d4e21fb246d31a2 compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=136f7e41e00000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=112b7c82e00000 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+92f32d4e21fb246d31a2@syzkaller.appspotmail.com BUG: kernel NULL pointer dereference, address: 0000000000000003 #PF: supervisor write access in kernel mode #PF: error_code(0x0002) - not-present page PGD a9a61067 P4D a9a61067 PUD 8fa24067 PMD 0 Oops: 0002 [#1] PREEMPT SMP KASAN CPU: 1 PID: 9054 Comm: syz-executor150 Not tainted 5.5.0-rc1-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 RIP: 0010:writew arch/x86/include/asm/io.h:66 [inline] RIP: 0010:mem16_serial_out+0x6c/0x90 drivers/tty/serial/8250/8250_port.c:414 Code: b6 8d e9 00 00 00 49 8d 7d 40 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 d3 e3 80 3c 02 00 75 19 48 63 db 49 03 5d 40 <66> 44 89 23 5b 41 5c 41 5d 5d c3 e8 d4 44 cf fd eb c2 e8 2d 45 cf RSP: 0018:ffffc90001cf7908 EFLAGS: 00010206 RAX: dffffc0000000000 RBX: 0000000000000003 RCX: 0000000000000000 RDX: 1ffffffff182080e RSI: ffffffff83e38106 RDI: ffffffff8c104070 RBP: ffffc90001cf7920 R08: ffff88808ffac040 R09: ffffed10431421c6 R10: ffffed10431421c5 R11: ffff888218a10e2b R12: 00000000000000bf R13: ffffffff8c104030 R14: ffffc90001cf7a40 R15: ffffffff8c104188 FS: 0000000000866880(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000003 CR3: 00000000a64a2000 CR4: 00000000001406e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: serial_port_out include/linux/serial_core.h:265 [inline] serial8250_do_startup+0x12b9/0x1cf0 drivers/tty/serial/8250/8250_port.c:2077 serial8250_startup+0x62/0x80 drivers/tty/serial/8250/8250_port.c:2329 uart_port_startup drivers/tty/serial/serial_core.c:219 [inline] uart_startup drivers/tty/serial/serial_core.c:258 [inline] uart_startup+0x452/0x980 drivers/tty/serial/serial_core.c:249 uart_set_info drivers/tty/serial/serial_core.c:998 [inline] uart_set_info_user+0x13b4/0x1cf0 drivers/tty/serial/serial_core.c:1023 tty_tiocsserial drivers/tty/tty_io.c:2506 [inline] tty_ioctl+0xf60/0x14f0 drivers/tty/tty_io.c:2648 vfs_ioctl fs/ioctl.c:47 [inline] file_ioctl fs/ioctl.c:545 [inline] do_vfs_ioctl+0x977/0x14e0 fs/ioctl.c:732 ksys_ioctl+0xab/0xd0 fs/ioctl.c:749 __do_sys_ioctl fs/ioctl.c:756 [inline] __se_sys_ioctl fs/ioctl.c:754 [inline] __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:754 do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x440219 Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:00007ffc99622388 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440219 RDX: 0000000020000240 RSI: 000000000000541f RDI: 0000000000000003 RBP: 00000000006ca018 R08: 0000000000000000 R09: 00000000004002c8 R10: 0000000000401b30 R11: 0000000000000246 R12: 0000000000401aa0 R13: 0000000000401b30 R14: 0000000000000000 R15: 0000000000000000 Modules linked in: CR2: 0000000000000003 ---[ end trace 2e0575eb0019173e ]--- RIP: 0010:writew arch/x86/include/asm/io.h:66 [inline] RIP: 0010:mem16_serial_out+0x6c/0x90 drivers/tty/serial/8250/8250_port.c:414 Code: b6 8d e9 00 00 00 49 8d 7d 40 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 d3 e3 80 3c 02 00 75 19 48 63 db 49 03 5d 40 <66> 44 89 23 5b 41 5c 41 5d 5d c3 e8 d4 44 cf fd eb c2 e8 2d 45 cf RSP: 0018:ffffc90001cf7908 EFLAGS: 00010206 RAX: dffffc0000000000 RBX: 0000000000000003 RCX: 0000000000000000 RDX: 1ffffffff182080e RSI: ffffffff83e38106 RDI: ffffffff8c104070 RBP: ffffc90001cf7920 R08: ffff88808ffac040 R09: ffffed10431421c6 R10: ffffed10431421c5 R11: ffff888218a10e2b R12: 00000000000000bf R13: ffffffff8c104030 R14: ffffc90001cf7a40 R15: ffffffff8c104188 FS: 0000000000866880(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000003 CR3: 00000000a64a2000 CR4: 00000000001406e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkaller@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#status for how to communicate with syzbot. syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: BUG: unable to handle kernel NULL pointer dereference in mem16_serial_out 2019-12-09 19:35 BUG: unable to handle kernel NULL pointer dereference in mem16_serial_out syzbot @ 2019-12-10 1:38 ` syzbot 2019-12-12 10:57 ` Greg KH 2021-04-26 16:14 ` [PATCH] serial: 8250: fix NULL pointer dereference in serial8250_do_startup() Vegard Nossum 1 sibling, 1 reply; 9+ messages in thread From: syzbot @ 2019-12-10 1:38 UTC (permalink / raw) To: andriy.shevchenko, asierra, corbet, ext-kimmo.rautkoski, gregkh, jslaby, kai.heng.feng, linux-api, linux-doc, linux-kernel, linux-serial, mika.westerberg, paulburton, peter, sr, syzkaller-bugs, yamada.masahiro, yegorslists syzbot has bisected this bug to: commit bd94c4077a0b2ecc35562c294f80f3659ecd8499 Author: Masahiro Yamada <yamada.masahiro@socionext.com> Date: Wed Oct 28 03:46:05 2015 +0000 serial: support 16-bit register interface for console bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=13723196e00000 start commit: e42617b8 Linux 5.5-rc1 git tree: upstream final crash: https://syzkaller.appspot.com/x/report.txt?x=10f23196e00000 console output: https://syzkaller.appspot.com/x/log.txt?x=17723196e00000 kernel config: https://syzkaller.appspot.com/x/.config?x=3754e2c78c1adb82 dashboard link: https://syzkaller.appspot.com/bug?extid=92f32d4e21fb246d31a2 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=136f7e41e00000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=112b7c82e00000 Reported-by: syzbot+92f32d4e21fb246d31a2@syzkaller.appspotmail.com Fixes: bd94c4077a0b ("serial: support 16-bit register interface for console") For information about bisection process see: https://goo.gl/tpsmEJ#bisection ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: BUG: unable to handle kernel NULL pointer dereference in mem16_serial_out 2019-12-10 1:38 ` syzbot @ 2019-12-12 10:57 ` Greg KH 2019-12-13 9:05 ` Dmitry Vyukov 0 siblings, 1 reply; 9+ messages in thread From: Greg KH @ 2019-12-12 10:57 UTC (permalink / raw) To: syzbot Cc: andriy.shevchenko, asierra, corbet, ext-kimmo.rautkoski, jslaby, kai.heng.feng, linux-api, linux-doc, linux-kernel, linux-serial, mika.westerberg, paulburton, peter, sr, syzkaller-bugs, yamada.masahiro, yegorslists On Mon, Dec 09, 2019 at 05:38:01PM -0800, syzbot wrote: > syzbot has bisected this bug to: > > commit bd94c4077a0b2ecc35562c294f80f3659ecd8499 > Author: Masahiro Yamada <yamada.masahiro@socionext.com> > Date: Wed Oct 28 03:46:05 2015 +0000 > > serial: support 16-bit register interface for console That would be because that is when this function was added to the kernel :) Again, you are asking the kernel to write to a bad place in memory, and then crash when that happens. That sounds like the correct functionality to me... thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: BUG: unable to handle kernel NULL pointer dereference in mem16_serial_out 2019-12-12 10:57 ` Greg KH @ 2019-12-13 9:05 ` Dmitry Vyukov 0 siblings, 0 replies; 9+ messages in thread From: Dmitry Vyukov @ 2019-12-13 9:05 UTC (permalink / raw) To: Greg KH Cc: syzbot, Andy Shevchenko, asierra, Jonathan Corbet, ext-kimmo.rautkoski, Jiri Slaby, kai heng feng, Linux API, open list:DOCUMENTATION, LKML, linux-serial, mika.westerberg, paulburton, Peter Hurley, sr, syzkaller-bugs, Masahiro Yamada, yegorslists On Thu, Dec 12, 2019 at 11:57 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Mon, Dec 09, 2019 at 05:38:01PM -0800, syzbot wrote: > > syzbot has bisected this bug to: > > > > commit bd94c4077a0b2ecc35562c294f80f3659ecd8499 > > Author: Masahiro Yamada <yamada.masahiro@socionext.com> > > Date: Wed Oct 28 03:46:05 2015 +0000 > > > > serial: support 16-bit register interface for console > > That would be because that is when this function was added to the kernel > :) > > Again, you are asking the kernel to write to a bad place in memory, and > then crash when that happens. That sounds like the correct > functionality to me... This looks like: #syz dup: BUG: unable to handle kernel NULL pointer dereference in mem_serial_out Let's continue in that thread. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] serial: 8250: fix NULL pointer dereference in serial8250_do_startup() 2019-12-09 19:35 BUG: unable to handle kernel NULL pointer dereference in mem16_serial_out syzbot 2019-12-10 1:38 ` syzbot @ 2021-04-26 16:14 ` Vegard Nossum 2021-04-26 16:17 ` Greg Kroah-Hartman 1 sibling, 1 reply; 9+ messages in thread From: Vegard Nossum @ 2021-04-26 16:14 UTC (permalink / raw) To: Greg Kroah-Hartman, linux-serial, syzbot+4c7f1a69dfe24c6b3aeb, syzbot+92f32d4e21fb246d31a2 Cc: linux-kernel, syzkaller-bugs, Vegard Nossum, Peter Hurley, Caleb Connolly Syzbot reported a crash, here reproduced on a recent mainline kernel: BUG: kernel NULL pointer dereference, address: 0000000000000005 #PF: supervisor read access in kernel mode #PF: error_code(0x0000) - not-present page PGD 120cf067 P4D 120cf067 PUD 135d4067 PMD 0 Oops: 0000 [#1] PREEMPT SMP KASAN CPU: 2 PID: 4830 Comm: a.out Not tainted 5.12.0-rc7+ #209 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014 RIP: 0010:mem16_serial_in+0x83/0xa0 [...] Call Trace: serial8250_do_startup+0x475/0x1e40 serial8250_startup+0x5c/0x80 uart_startup+0x360/0x870 uart_set_info_user+0x13a3/0x1c30 tty_ioctl+0x711/0x14f0 __x64_sys_ioctl+0x193/0x200 do_syscall_64+0x2d/0x70 entry_SYSCALL_64_after_hwframe+0x44/0xae A more readable reproducer is: #include <sys/ioctl.h> #include <fcntl.h> #include <linux/serial.h> #ifndef SERIAL_IO_MEM16 #define SERIAL_IO_MEM16 7 #endif int main(int argc, char *argv[]) { int fd = open("/dev/ttyS3", O_RDONLY); struct serial_struct ss = {}; ss.type = 0x10; ss.baud_base = 0x7fffffff; ss.io_type = SERIAL_IO_MEM16; ioctl(fd, TIOCSSERIAL, &ss); return 0; } ioctl(TIOCSSERIAL) attempts to configure the serial port, but when requesting io_type SERIAL_IO_MEM*/UPIO_MEM* it goes on to dereference ->membase in serial8250_do_startup(). I propose this fix, which will fail validation of the TIOCSSERIAL request if you request a memory-based or io-based io_type when the underlying port has no valid ->membase or ->iobase, respectively. As far as I can tell, this driver was written to support being able to switch between the two IO types for a given port (assuming the underlying driver supports it); see serial8250_do_startup()/set_io_from_upio(). I'm also adding a couple of WARN_ON_ONCE()s which are technically redundant, but which could help somebody else if they come across a similar issue in the future. Reported-by: syzbot+4c7f1a69dfe24c6b3aeb@syzkaller.appspotmail.com Cc: Peter Hurley <peter@hurleysoftware.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: linux-serial@vger.kernel.org Cc: Caleb Connolly <caleb@connolly.tech> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com> --- drivers/tty/serial/8250/8250_port.c | 43 +++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index b0af13074cd36..aec3abff8e48e 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -455,6 +455,33 @@ static void io_serial_out(struct uart_port *p, int offset, int value) static int serial8250_default_handle_irq(struct uart_port *port); +static int needs_membase(int iotype) +{ + switch (iotype) { + case UPIO_MEM: + case UPIO_MEM16: + case UPIO_MEM32: + case UPIO_MEM32BE: +#ifdef CONFIG_SERIAL_8250_RT288X + case UPIO_AU: +#endif + return 1; + } + + return 0; +} + +static int needs_iobase(int iotype) +{ + switch (iotype) { + case UPIO_HUB6: + case UPIO_PORT: + return 1; + } + + return 0; +} + static void set_io_from_upio(struct uart_port *p) { struct uart_8250_port *up = up_to_u8250p(p); @@ -2151,6 +2178,11 @@ int serial8250_do_startup(struct uart_port *port) unsigned char lsr, iir; int retval; + if (WARN_ON_ONCE(needs_membase(port->iotype) && !port->membase)) + return -ENODEV; + if (WARN_ON_ONCE(needs_iobase(port->iotype) && !port->iobase)) + return -ENODEV; + if (!port->fifosize) port->fifosize = uart_config[port->type].fifo_size; if (!up->tx_loadsz) @@ -3157,6 +3189,17 @@ serial8250_verify_port(struct uart_port *port, struct serial_struct *ser) ser->type >= ARRAY_SIZE(uart_config) || ser->type == PORT_CIRRUS || ser->type == PORT_STARTECH) return -EINVAL; + + /* + * This driver clearly was intended to support switching between + * io types (see serial8250_do_startup()), so we need to ensure that + * the underlying port type will support the request. + */ + if (needs_membase(ser->io_type) && !port->membase) + return -EINVAL; + if (needs_iobase(ser->io_type) && !port->iobase) + return -EINVAL; + return 0; } -- 2.16.1.72.g5be1f00a9.dirty ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] serial: 8250: fix NULL pointer dereference in serial8250_do_startup() 2021-04-26 16:14 ` [PATCH] serial: 8250: fix NULL pointer dereference in serial8250_do_startup() Vegard Nossum @ 2021-04-26 16:17 ` Greg Kroah-Hartman 2021-04-26 16:33 ` Vegard Nossum 0 siblings, 1 reply; 9+ messages in thread From: Greg Kroah-Hartman @ 2021-04-26 16:17 UTC (permalink / raw) To: Vegard Nossum Cc: linux-serial, syzbot+4c7f1a69dfe24c6b3aeb, syzbot+92f32d4e21fb246d31a2, linux-kernel, syzkaller-bugs, Peter Hurley, Caleb Connolly On Mon, Apr 26, 2021 at 06:14:33PM +0200, Vegard Nossum wrote: > Syzbot reported a crash, here reproduced on a recent mainline kernel: > > BUG: kernel NULL pointer dereference, address: 0000000000000005 > #PF: supervisor read access in kernel mode > #PF: error_code(0x0000) - not-present page > PGD 120cf067 P4D 120cf067 PUD 135d4067 PMD 0 > Oops: 0000 [#1] PREEMPT SMP KASAN > CPU: 2 PID: 4830 Comm: a.out Not tainted 5.12.0-rc7+ #209 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014 > RIP: 0010:mem16_serial_in+0x83/0xa0 > [...] > Call Trace: > serial8250_do_startup+0x475/0x1e40 > serial8250_startup+0x5c/0x80 > uart_startup+0x360/0x870 > uart_set_info_user+0x13a3/0x1c30 > tty_ioctl+0x711/0x14f0 > __x64_sys_ioctl+0x193/0x200 > do_syscall_64+0x2d/0x70 > entry_SYSCALL_64_after_hwframe+0x44/0xae > > A more readable reproducer is: > > #include <sys/ioctl.h> > #include <fcntl.h> > > #include <linux/serial.h> > > #ifndef SERIAL_IO_MEM16 > #define SERIAL_IO_MEM16 7 > #endif > > int main(int argc, char *argv[]) > { > int fd = open("/dev/ttyS3", O_RDONLY); > > struct serial_struct ss = {}; > ss.type = 0x10; > ss.baud_base = 0x7fffffff; > ss.io_type = SERIAL_IO_MEM16; > ioctl(fd, TIOCSSERIAL, &ss); > > return 0; > } > > ioctl(TIOCSSERIAL) attempts to configure the serial port, but when > requesting io_type SERIAL_IO_MEM*/UPIO_MEM* it goes on to dereference > ->membase in serial8250_do_startup(). > > I propose this fix, which will fail validation of the TIOCSSERIAL request > if you request a memory-based or io-based io_type when the underlying port > has no valid ->membase or ->iobase, respectively. > > As far as I can tell, this driver was written to support being able to > switch between the two IO types for a given port (assuming the underlying > driver supports it); see serial8250_do_startup()/set_io_from_upio(). > > I'm also adding a couple of WARN_ON_ONCE()s which are technically > redundant, but which could help somebody else if they come across a > similar issue in the future. > > Reported-by: syzbot+4c7f1a69dfe24c6b3aeb@syzkaller.appspotmail.com > Cc: Peter Hurley <peter@hurleysoftware.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: linux-serial@vger.kernel.org > Cc: Caleb Connolly <caleb@connolly.tech> > Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com> > --- > drivers/tty/serial/8250/8250_port.c | 43 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 43 insertions(+) > > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c > index b0af13074cd36..aec3abff8e48e 100644 > --- a/drivers/tty/serial/8250/8250_port.c > +++ b/drivers/tty/serial/8250/8250_port.c > @@ -455,6 +455,33 @@ static void io_serial_out(struct uart_port *p, int offset, int value) > > static int serial8250_default_handle_irq(struct uart_port *port); > > +static int needs_membase(int iotype) > +{ > + switch (iotype) { > + case UPIO_MEM: > + case UPIO_MEM16: > + case UPIO_MEM32: > + case UPIO_MEM32BE: > +#ifdef CONFIG_SERIAL_8250_RT288X > + case UPIO_AU: > +#endif > + return 1; > + } > + > + return 0; > +} > + > +static int needs_iobase(int iotype) > +{ > + switch (iotype) { > + case UPIO_HUB6: > + case UPIO_PORT: > + return 1; > + } > + > + return 0; > +} > + > static void set_io_from_upio(struct uart_port *p) > { > struct uart_8250_port *up = up_to_u8250p(p); > @@ -2151,6 +2178,11 @@ int serial8250_do_startup(struct uart_port *port) > unsigned char lsr, iir; > int retval; > > + if (WARN_ON_ONCE(needs_membase(port->iotype) && !port->membase)) > + return -ENODEV; > + if (WARN_ON_ONCE(needs_iobase(port->iotype) && !port->iobase)) > + return -ENODEV; These WARN_ON() will still trigger syzbot. Are you sure you tested this and had syzbot verify it? thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] serial: 8250: fix NULL pointer dereference in serial8250_do_startup() 2021-04-26 16:17 ` Greg Kroah-Hartman @ 2021-04-26 16:33 ` Vegard Nossum 2021-04-28 6:36 ` BUG: unable to handle kernel NULL pointer dereference in mem16_serial_out syzbot 2021-05-13 14:24 ` [PATCH] serial: 8250: fix NULL pointer dereference in serial8250_do_startup() Greg Kroah-Hartman 0 siblings, 2 replies; 9+ messages in thread From: Vegard Nossum @ 2021-04-26 16:33 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-serial, syzbot+4c7f1a69dfe24c6b3aeb, syzbot+92f32d4e21fb246d31a2, linux-kernel, syzkaller-bugs, Peter Hurley, Caleb Connolly On 2021-04-26 18:17, Greg Kroah-Hartman wrote: > On Mon, Apr 26, 2021 at 06:14:33PM +0200, Vegard Nossum wrote: >> static void set_io_from_upio(struct uart_port *p) >> { >> struct uart_8250_port *up = up_to_u8250p(p); >> @@ -2151,6 +2178,11 @@ int serial8250_do_startup(struct uart_port *port) >> unsigned char lsr, iir; >> int retval; >> >> + if (WARN_ON_ONCE(needs_membase(port->iotype) && !port->membase)) >> + return -ENODEV; >> + if (WARN_ON_ONCE(needs_iobase(port->iotype) && !port->iobase)) >> + return -ENODEV; > > These WARN_ON() will still trigger syzbot. Are you sure you tested this > and had syzbot verify it? I tested it locally and the WARN_ON()s don't trigger -- presumably because serial8250_verify_port() is called from uart_set_info() before we get to serial8250_do_startup(): /* * Ask the low level driver to verify the settings. */ if (uport->ops->verify_port) retval = uport->ops->verify_port(uport, new_info); [...] retval = uart_startup(tty, state, 1); At least, this was my intention. Although now that I look at it again, it looks like this check may be skipped in some cases; is that what you're referring to? I didn't have syzbot verify it -- I thought it would do that when submitting my patch. Looks like I need to push somewhere and ask syzbot to test it using this? #syz test: git://repo/address.git commit-hash (I assume I can send this privately as long as I use the right syzbot+...@ To-address?) Thanks, Vegard ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: BUG: unable to handle kernel NULL pointer dereference in mem16_serial_out 2021-04-26 16:33 ` Vegard Nossum @ 2021-04-28 6:36 ` syzbot 2021-05-13 14:24 ` [PATCH] serial: 8250: fix NULL pointer dereference in serial8250_do_startup() Greg Kroah-Hartman 1 sibling, 0 replies; 9+ messages in thread From: syzbot @ 2021-04-28 6:36 UTC (permalink / raw) To: caleb, gregkh, linux-kernel, linux-serial, peter, syzkaller-bugs, syzkaller-lts-bugs, vegard.nossum Hello, syzbot tried to test the proposed patch but the build/boot failed: failed to checkout kernel repo git://repo/address.git/commit-hash: failed to run ["git" "fetch" "--force" "b7cf8f2fbfc36c709a08e0b9c77990e491473738" "commit-hash"]: exit status 128 fatal: unable to look up repo (port 9418) (Name or service not known) Tested on: commit: [unknown git tree: git://repo/address.git commit-hash dashboard link: https://syzkaller.appspot.com/bug?extid=4c7f1a69dfe24c6b3aeb compiler: ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] serial: 8250: fix NULL pointer dereference in serial8250_do_startup() 2021-04-26 16:33 ` Vegard Nossum 2021-04-28 6:36 ` BUG: unable to handle kernel NULL pointer dereference in mem16_serial_out syzbot @ 2021-05-13 14:24 ` Greg Kroah-Hartman 1 sibling, 0 replies; 9+ messages in thread From: Greg Kroah-Hartman @ 2021-05-13 14:24 UTC (permalink / raw) To: Vegard Nossum Cc: linux-serial, syzbot+4c7f1a69dfe24c6b3aeb, syzbot+92f32d4e21fb246d31a2, linux-kernel, syzkaller-bugs, Peter Hurley, Caleb Connolly On Mon, Apr 26, 2021 at 06:33:01PM +0200, Vegard Nossum wrote: > On 2021-04-26 18:17, Greg Kroah-Hartman wrote: > > On Mon, Apr 26, 2021 at 06:14:33PM +0200, Vegard Nossum wrote: > > > static void set_io_from_upio(struct uart_port *p) > > > { > > > struct uart_8250_port *up = up_to_u8250p(p); > > > @@ -2151,6 +2178,11 @@ int serial8250_do_startup(struct uart_port *port) > > > unsigned char lsr, iir; > > > int retval; > > > + if (WARN_ON_ONCE(needs_membase(port->iotype) && !port->membase)) > > > + return -ENODEV; > > > + if (WARN_ON_ONCE(needs_iobase(port->iotype) && !port->iobase)) > > > + return -ENODEV; > > > > These WARN_ON() will still trigger syzbot. Are you sure you tested this > > and had syzbot verify it? > > I tested it locally and the WARN_ON()s don't trigger -- presumably > because serial8250_verify_port() is called from uart_set_info() before > we get to serial8250_do_startup(): > > /* > * Ask the low level driver to verify the settings. > */ > if (uport->ops->verify_port) > retval = uport->ops->verify_port(uport, new_info); > > [...] > > retval = uart_startup(tty, state, 1); > > At least, this was my intention. Although now that I look at it again, > it looks like this check may be skipped in some cases; is that what > you're referring to? > > I didn't have syzbot verify it -- I thought it would do that when > submitting my patch. Looks like I need to push somewhere and ask syzbot > to test it using this? > > #syz test: git://repo/address.git commit-hash > > (I assume I can send this privately as long as I use the right > syzbot+...@ To-address?) > Dropping this now until you get this tested properly... thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-05-13 14:24 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-09 19:35 BUG: unable to handle kernel NULL pointer dereference in mem16_serial_out syzbot 2019-12-10 1:38 ` syzbot 2019-12-12 10:57 ` Greg KH 2019-12-13 9:05 ` Dmitry Vyukov 2021-04-26 16:14 ` [PATCH] serial: 8250: fix NULL pointer dereference in serial8250_do_startup() Vegard Nossum 2021-04-26 16:17 ` Greg Kroah-Hartman 2021-04-26 16:33 ` Vegard Nossum 2021-04-28 6:36 ` BUG: unable to handle kernel NULL pointer dereference in mem16_serial_out syzbot 2021-05-13 14:24 ` [PATCH] serial: 8250: fix NULL pointer dereference in serial8250_do_startup() Greg Kroah-Hartman
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).