linux-kernel - Re: [PATCH v2 1/3] vt: fix check for system/busy console drivers when unregistering them

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]

Date:	Mon, 15 Dec 2014 16:05:31 +0100
From:	Daniel Vetter <daniel.vetter@...ll.ch>
To:	Imre Deak <imre.deak@...el.com>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jslaby@...e.cz>,
	Peter Hurley <peter@...leysoftware.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/3] vt: fix check for system/busy console drivers when
 unregistering them

This seems to partially revert

commit d9c660e750fdf982e1e2bdd0e76c1e6c4db4217b
Author: Daniel Vetter <daniel.vetter@...ll.ch>
Date:   Thu Jun 5 16:29:56 2014 +0200

    vt: Fix up unregistration of vt drivers

    A bunch of issues:
    - We should not kick out the default console (which is tracked in
      conswitchp), so check for that.
    - Add better error codes so callers can differentiate between "something
      went wrong" and "your driver isn't registered already". i915 needs
      that so it doesn't fall over when reloading the driver and hence
      vga_con is already unregistered.
    - There's a mess with the driver flags: What we need to check for is
      that the driver isn't used any more, i.e. unbound completely (FLAG_INIT).
      And not whether it's the boot console or not (which is the only one
      which doesn't have FLAG_MODULE). Otherwise there's no way to kick
      out the boot console, which i915 wants to do to prevent havoc with
      vga_con interferring (which tends to hang machines).

    Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
    Cc: Jiri Slaby <jslaby@...e.cz>
    Reviewed-by: David Herrmann <dh.herrmann@...il.com>
    Acked-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
    Signed-off-by: Daniel Vetter <daniel.vetter@...ll.ch>

We really need to unregister vgacon when i915 loads since we
completely kill vga support - just unbinding is not enough since then
vgacon will be resurrect on module unload, killing the machine. And
module unload is really important to stay sane as kernel driver
hacker.

If we need more precise checks for unregistering then I think we need
to fix up that mess with the flags ...
-Daniel


On Sat, Dec 13, 2014 at 10:14 PM, Imre Deak <imre.deak@...el.com> wrote:
> System console drivers (without the CON_DRIVER_FLAG_MODULE flag) and
> busy drivers bound to a console (as reported by con_is_bound())
> shouldn't be unregistered. The current code checks for the
> CON_DRIVER_FLAG_INIT flag but this doesn't really correspond to either
> of the above two conditions. CON_DRIVER_FLAG_INIT is set whenever its
> associated console's con_startup() function is called, which first
> happens when the console driver is registered (so before the console
> gets bound) and gets cleared when the console gets unbound. The
> purpose of this flag is to show if we need to call con_startup() on a
> console before we use it.
>
> Based on the above, do_unregister_con_driver() in its current form will
> incorrectly allow unregistering a console driver only if it was never
> bound, but will refuse to unregister one that was bound and later
> unbound. It will also allow unregistering a system console driver.
>
> Fix this by checking for CON_DRIVER_FLAG_MODULE to allow non-system
> console drivers to unregister and prevent system console drivers from
> unregistering. Rely on the existing con_is_bound() check earlier in the
> function to refuse unregistering a busy console driver.
>
> v2:
> - reword the third paragraph to clarify how the fix works (Peter Hurley)
>
> Signed-off-by: Imre Deak <imre.deak@...el.com>
> ---
>  drivers/tty/vt/vt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index b33b00b..1862e89 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -3660,7 +3660,7 @@ int do_unregister_con_driver(const struct consw *csw)
>                 struct con_driver *con_driver = &registered_con_driver[i];
>
>                 if (con_driver->con == csw &&
> -                   con_driver->flag & CON_DRIVER_FLAG_INIT) {
> +                   con_driver->flag & CON_DRIVER_FLAG_MODULE) {
>                         vtconsole_deinit_device(con_driver);
>                         device_destroy(vtconsole_class,
>                                        MKDEV(0, con_driver->node));
> --
> 1.8.4
>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
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/