From: Vegard Nossum <vegard.nossum@oracle.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-serial@vger.kernel.org,
syzbot+4c7f1a69dfe24c6b3aeb@syzkaller.appspotmail.com,
syzbot+92f32d4e21fb246d31a2@syzkaller.appspotmail.com
Cc: linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com,
Vegard Nossum <vegard.nossum@oracle.com>,
Peter Hurley <peter@hurleysoftware.com>,
Caleb Connolly <caleb@connolly.tech>
Subject: [PATCH] serial: 8250: fix NULL pointer dereference in serial8250_do_startup()
Date: Mon, 26 Apr 2021 18:14:33 +0200 [thread overview]
Message-ID: <20210426161433.20829-1-vegard.nossum@oracle.com> (raw)
In-Reply-To: <00000000000044a65205994a7e13@google.com>
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
next prev parent reply other threads:[~2021-04-26 16:15 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
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 ` Vegard Nossum [this message]
2021-04-26 16:17 ` [PATCH] serial: 8250: fix NULL pointer dereference in serial8250_do_startup() 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210426161433.20829-1-vegard.nossum@oracle.com \
--to=vegard.nossum@oracle.com \
--cc=caleb@connolly.tech \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=peter@hurleysoftware.com \
--cc=syzbot+4c7f1a69dfe24c6b3aeb@syzkaller.appspotmail.com \
--cc=syzbot+92f32d4e21fb246d31a2@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).