linux-kernel - Re: [PATCH v3 3/3] Use separate struct console structure for each hvc

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: <1320825912.9376.72.camel@concordia>
Date:	Wed, 09 Nov 2011 19:05:12 +1100
From:	Michael Ellerman <michael@...erman.id.au>
To:	Miche Baker-Harvey <miche@...gle.com>
Cc:	Greg Kroah-Hartman <gregkh@...e.de>,
	Stephen Rothwell <sfr@...b.auug.org.au>,
	xen-devel@...ts.xensource.com,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Rusty Russell <rusty@...tcorp.com.au>,
	linux-kernel@...r.kernel.org,
	virtualization@...ts.linux-foundation.org,
	Anton Blanchard <anton@...ba.org>,
	Amit Shah <amit.shah@...hat.com>,
	Mike Waychison <mikew@...gle.com>,
	ppc-dev <linuxppc-dev@...ts.ozlabs.org>,
	Eric Northrup <digitaleric@...gle.com>
Subject: Re: [PATCH v3 3/3] Use separate struct console structure for each
 hvc_console.

On Tue, 2011-11-08 at 13:45 -0800, Miche Baker-Harvey wrote:
> It is possible to make any virtio_console port be a console
> by sending VIRITO_CONSOLE_CONSOLE_PORT.  But hvc_alloc was
> using a single struct console hvc_console, which contains
> both an index and flags which are per-port.
> 
> This adds a separate struct console for each virtio_console
> that is CONSOLE_PORT.

Hi Miche,

I'm testing this on powerpc and unfortunately it's working a little _too
well_. I end up with two struct consoles registered and so I get every
line of output twice :)

The problem is that we're registering two struct consoles. The first
obviously is hvc_console, either in hvc_console_init(), or in my case
from hvc_instantiate().

Then we register the allocated one in hvc_alloc(). But because they both
point back to the same hardware you get duplicate output.

We _do_ want to register a console early, in either/both
hvc_console_init() and hvc_instantiate(), because we want to have
console during boot prior to when hvc_alloc() gets called.

I think maybe we should be checking in hvc_alloc() whether we already
have hvc_console associated with the vtermno and if so we use
hvc_console instead of allocating a new one.

Patch below to do that, and works for me, but it's a bit of a hack,
there must be a better solution.

Finally I'm not sure how your patch affects the code in hvc_poll() which
checks hvc_console.index to do the SYSRQ hack.

cheers

diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index fff35da..b249195 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -815,13 +815,15 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
        kref_init(&hp->kref);
 
        INIT_WORK(&hp->tty_resize, hvc_set_winsz);
-       /*
-        * Make each console its own struct console.
-        */
-       cp = kmemdup(&hvc_console, sizeof(*cp), GFP_KERNEL);
-       if (!cp) {
-               kfree(hp);
-               return ERR_PTR(-ENOMEM);
+
+       if (hvc_console.index >= 0 && vtermnos[hvc_console.index] == hp->vtermno)
+               cp = &hvc_console;
+       else {
+               cp = kmemdup(&hvc_console, sizeof(*cp), GFP_KERNEL);
+               if (!cp) {
+                       kfree(hp);
+                       return ERR_PTR(-ENOMEM);
+               }
        }
 
        hp->hvc_console = cp;
@@ -850,7 +852,9 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
 
        list_add_tail(&(hp->next), &hvc_structs);
        spin_unlock(&hvc_structs_lock);
-       register_console(cp);
+
+       if (cp != &hvc_console)
+               register_console(cp);
 
        return hp;
 }


Download attachment "signature.asc" of type "application/pgp-signature" (199 bytes)