Re: [PATCH] serial: 8250: fix NULL pointer dereference in serial8250_do_startup()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux