[PATCH v2 0/2] fix out-of-bounds access when specifying invalid console

[PATCH v2 0/2] fix out-of-bounds access when specifying invalid console

[Sven Schnelle]

Hi,

these two patches fix a crash in the tty driver when a user specifies an
invalid console like 'console=tty3000'. The first patch adds a check to
tty_driver_lookup_tty(), the second one prevents that such a console gets
registered in the vt driver.

Changes in v2:
- trim commit message in first patch
- add second patch as suggested by Jiri Slaby

Sven Schnelle (2):
  tty: fix out-of-bounds access in tty_driver_lookup_tty()
  tty/vt: prevent registration of console with invalid number

 drivers/tty/tty_io.c | 8 +++++---
 drivers/tty/vt/vt.c  | 6 ++++++
 2 files changed, 11 insertions(+), 3 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] tty: fix out-of-bounds access in tty_driver_lookup_tty()

[Sven Schnelle]

When specifying an invalid console= device like console=tty3270,
tty_driver_lookup_tty() returns the tty struct without checking
whether index is a valid number.

To reproduce:

qemu-system-x86_64 -enable-kvm -nographic -serial mon:stdio \
-kernel ../linux-build-x86/arch/x86/boot/bzImage \
-append "console=ttyS0 console=tty3270"

This crashes with:

[    0.770599] BUG: kernel NULL pointer dereference, address: 00000000000000ef
[    0.771265] #PF: supervisor read access in kernel mode
[    0.771773] #PF: error_code(0x0000) - not-present page
[    0.772609] Oops: 0000 [#1] PREEMPT SMP PTI
[    0.774878] RIP: 0010:tty_open+0x268/0x6f0
[    0.784013]  chrdev_open+0xbd/0x230
[    0.784444]  ? cdev_device_add+0x80/0x80
[    0.784920]  do_dentry_open+0x1e0/0x410
[    0.785389]  path_openat+0xca9/0x1050
[    0.785813]  do_filp_open+0xaa/0x150
[    0.786240]  file_open_name+0x133/0x1b0
[    0.786746]  filp_open+0x27/0x50
[    0.787244]  console_on_rootfs+0x14/0x4d
[    0.787800]  kernel_init_freeable+0x1e4/0x20d
[    0.788383]  ? rest_init+0xc0/0xc0
[    0.788881]  kernel_init+0x11/0x120
[    0.789356]  ret_from_fork+0x22/0x30

Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
Reviewed-by: Jiri Slaby <jirislaby@kernel.org>
---
 drivers/tty/tty_io.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index de06c3c2ff70..1ac6784ea1f9 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1224,14 +1224,16 @@ static struct tty_struct *tty_driver_lookup_tty(struct tty_driver *driver,
 {
 	struct tty_struct *tty;
 
-	if (driver->ops->lookup)
+	if (driver->ops->lookup) {
 		if (!file)
 			tty = ERR_PTR(-EIO);
 		else
 			tty = driver->ops->lookup(driver, file, idx);
-	else
+	} else {
+		if (idx >= driver->num)
+			return ERR_PTR(-EINVAL);
 		tty = driver->ttys[idx];
-
+	}
 	if (!IS_ERR(tty))
 		tty_kref_get(tty);
 	return tty;
-- 
2.34.1

[PATCH v2 2/2] tty/vt: prevent registration of console with invalid number

[Sven Schnelle]

If a user specifies an invalid console like 'console=tty3000',
the vt driver should prevent setting up a vt entry for that.

Suggested-by: Jiri Slaby <jirislaby@kernel.org>
Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
---
 drivers/tty/vt/vt.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 981d2bfcf9a5..62c8a45ad731 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -3156,8 +3156,14 @@ static struct tty_driver *vt_console_device(struct console *c, int *index)
 	return console_driver;
 }
 
+static int vt_console_setup(struct console *co, char *options)
+{
+	return co->index >= MAX_NR_CONSOLES ? -EINVAL : 0;
+}
+
 static struct console vt_console_driver = {
 	.name		= "tty",
+	.setup		= vt_console_setup,
 	.write		= vt_console_print,
 	.device		= vt_console_device,
 	.unblank	= unblank_screen,
-- 
2.34.1

Re: [PATCH v2 0/2] fix out-of-bounds access when specifying invalid console

[Sven Schnelle]

Sven Schnelle <svens@linux.ibm.com> writes:

> Hi,
>
> these two patches fix a crash in the tty driver when a user specifies an
> invalid console like 'console=tty3000'. The first patch adds a check to
> tty_driver_lookup_tty(), the second one prevents that such a console gets
> registered in the vt driver.
>
> Changes in v2:
> - trim commit message in first patch
> - add second patch as suggested by Jiri Slaby
>
> Sven Schnelle (2):
>   tty: fix out-of-bounds access in tty_driver_lookup_tty()
>   tty/vt: prevent registration of console with invalid number
>
>  drivers/tty/tty_io.c | 8 +++++---
>  drivers/tty/vt/vt.c  | 6 ++++++
>  2 files changed, 11 insertions(+), 3 deletions(-)

Gentle ping... I couldn't find that this was applied anywhere?

Thanks
Sven

Re: [PATCH v2 0/2] fix out-of-bounds access when specifying invalid console

[Greg Kroah-Hartman]

On Wed, Jan 04, 2023 at 08:20:49AM +0100, Sven Schnelle wrote:
> Sven Schnelle <svens@linux.ibm.com> writes:
> 
> > Hi,
> >
> > these two patches fix a crash in the tty driver when a user specifies an
> > invalid console like 'console=tty3000'. The first patch adds a check to
> > tty_driver_lookup_tty(), the second one prevents that such a console gets
> > registered in the vt driver.
> >
> > Changes in v2:
> > - trim commit message in first patch
> > - add second patch as suggested by Jiri Slaby
> >
> > Sven Schnelle (2):
> >   tty: fix out-of-bounds access in tty_driver_lookup_tty()
> >   tty/vt: prevent registration of console with invalid number
> >
> >  drivers/tty/tty_io.c | 8 +++++---
> >  drivers/tty/vt/vt.c  | 6 ++++++
> >  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> Gentle ping... I couldn't find that this was applied anywhere?

It's in my to-review queue, which is about 2000+ patches right now, give
me some time to dig it out.  In the meantime, please feel free to review
other pending patches on the list to help out with the workload.

thanks,

greg k-h

Re: [PATCH v2 0/2] fix out-of-bounds access when specifying invalid console

[Greg Kroah-Hartman]

On Wed, Jan 04, 2023 at 08:20:49AM +0100, Sven Schnelle wrote:
> Sven Schnelle <svens@linux.ibm.com> writes:
> 
> > Hi,
> >
> > these two patches fix a crash in the tty driver when a user specifies an
> > invalid console like 'console=tty3000'. The first patch adds a check to
> > tty_driver_lookup_tty(), the second one prevents that such a console gets
> > registered in the vt driver.
> >
> > Changes in v2:
> > - trim commit message in first patch
> > - add second patch as suggested by Jiri Slaby
> >
> > Sven Schnelle (2):
> >   tty: fix out-of-bounds access in tty_driver_lookup_tty()
> >   tty/vt: prevent registration of console with invalid number
> >
> >  drivers/tty/tty_io.c | 8 +++++---
> >  drivers/tty/vt/vt.c  | 6 ++++++
> >  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> Gentle ping... I couldn't find that this was applied anywhere?

Sorry, still digging out of a lot of patches...