Messages in this thread | | | Subject | Re: [PATCH] serial: 8250: fix NULL pointer dereference in serial8250_do_startup() | From | Vegard Nossum <> | Date | Mon, 26 Apr 2021 18:33:01 +0200 |
| |
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
| |