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
