linux-kernel - Re: serial167 (was: Re: [RFC 0/5] tty: move stuff around)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110317104618.095f16ba@lxorguk.ukuu.org.uk>
Date:	Thu, 17 Mar 2011 10:46:18 +0000
From:	Alan Cox <alan@...rguk.ukuu.org.uk>
To:	Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:	Arnd Bergmann <arnd@...db.de>, Greg KH <gregkh@...e.de>,
	Jiri Slaby <jslaby@...e.cz>, linux-kernel@...r.kernel.org,
	Rogier Wolff <R.E.Wolff@...wizard.nl>,
	"Michael H. Warfield" <mhw@...tsend.com>,
	"Linux/m68k" <linux-m68k@...r.kernel.org>
Subject: Re: serial167 (was: Re: [RFC 0/5] tty: move stuff around)

> As serial167 is now in staging in Linus tree...
> 
> It still builds in 2.6.38. What is unsecure about it? If nothing,
> please move it back,
> as the whole MVME167 platform is pretty useless without its console driver.

It has no reference counting and it doesn't use tty_port objects.

The former has been required for many releases otherwise your tty objects
are going to vanish under you and stuff, the latter is now used by all
'active' drivers and once we have total coverage will allow a lot of
simplification as it will mean a lot of structures that belong to the
physical port lifetime can be put there - like the tty input buffers.

You have other problems too - eg startup and shutdown depend upon the big
kernel lock - which is now gone from those areas.

The tty_port helpers now in the kernel will do a lot of the work for you.
They are designed to do all the locking and nastiness around
open/close/hangup and provide you with serialized simple handling for
startup/shutdown.

At minimum you want to add tty refcounting and tty port objects. If you
use the tty_port helpers for open/close/hangup and rip out all but the
tiny core bits of startup/shutdown etc that should also sort out the main
other problems.

Going via info->tty can be done via tty_port_tty_get() but in this driver
you can probably prove it's safe to take a kref when you assign the
port->tty field and drop it on shutdown at least for the IRQ paths, for
some of the none IRQ paths it looks like tty should be passed down. I am
not sure about user triggered vhangup() being ok to just keep the single
ref. tty_port_tty_get() at the start of the IRQ handler, passing the
tty reference around and putting the kref at the end would be a fraction
heavier but should be bulletproof.

Bringing it up to the 21st century might not be a bad idea either -
consts, platform device, no hardcoded board specific hackery, not
essential but if the rest of the driver is getting a much needed service
perhaps it's worth treating as it was a new submission and giving it a
full review and cleanup to modern spec ? We wouldn't accept the existing
driver as a submission today.

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/