linux-kernel - Re: [Xen-devel] [PATCH] xen/hvc: Fix polling mode to work with kdb/kgdb

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:	Thu, 7 Jun 2012 16:07:40 -0400
From:	Ben Guthro <ben@...hro.net>
To:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
Cc:	xen-devel@...ts.xensource.com, linux-kernel@...r.kernel.org
Subject: Re: [Xen-devel] [PATCH] xen/hvc: Fix polling mode to work with kdb/kgdb

On Thu, Jun 7, 2012 at 2:01 PM, Konrad Rzeszutek Wilk
<konrad.wilk@...cle.com> wrote:
<snip>
>> I'll try to dig in a bit, and report back.
>
> OK. Thanks!

OK, after throwing a bit of trace in, I remember now, the issue I was
working around.

The tty layer does not seem keep the console open.
It opens, and closes the tty based on whether there is data to print.

However, the CONFIG_CONSOLE_POLL code goes around all of this, and
uses the polling capabilities of the driver.
Most of the drivers that implement this mechanism are serial devices
where they are actually polling the UART.

When Alt+SysRq+g is pressed, the output is flushed, all references to
the hvc are released, and then hvc_close() is called.

However, since this code fully bypasses the tty layer, no reference is
grabbed - and so, when we come into hvc_poll_put_char, and the
necessary structures aren't available.

Below is an updated version of this patch that will use the
tty->driver_data, if available - but will fall back to using cons_ops.

This will guarantee that any existing users of hvc where the above
claim is not true will see no change in behavior.

However...The part of this that I don't really understand why we need
is in kernel/debug/debug_core.c
I don't like having to put the ifdef in there. I was hoping to be able
to remove it, with the upstreaming of the xen IPI code... but in
testing, it seems to still be necessary.

diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index 2d691eb..7f965e1 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -767,11 +767,14 @@ int hvc_poll_init(struct tty_driver *driver, int
line, char *options)
 static int hvc_poll_get_char(struct tty_driver *driver, int line)
 {
        struct tty_struct *tty = driver->ttys[0];
-       struct hvc_struct *hp = tty->driver_data;
+       struct hvc_struct *hp = tty ? tty->driver_data : NULL;
+       struct hv_ops *ops = (hp && hp->ops) ? hp->ops : cons_ops[last_hvc];
+       uint32_t vtno = hp ? hp->vtermno : vtermnos[last_hvc];
+
        int n;
        char ch;

-       n = hp->ops->get_chars(hp->vtermno, &ch, 1);
+       n = ops->get_chars(vtno, &ch, 1);

        if (n == 0)
                return NO_POLL_CHAR;
@@ -782,11 +785,14 @@ static int hvc_poll_get_char(struct tty_driver
*driver, int line)
 static void hvc_poll_put_char(struct tty_driver *driver, int line, char ch)
 {
        struct tty_struct *tty = driver->ttys[0];
-       struct hvc_struct *hp = tty->driver_data;
+       struct hvc_struct *hp = tty ? tty->driver_data : NULL;
+       struct hv_ops *ops = (hp && hp->ops) ? hp->ops : cons_ops[last_hvc];
+       uint32_t vtno = hp ? hp->vtermno : vtermnos[last_hvc];
+
        int n;

        do {
-               n = hp->ops->put_chars(hp->vtermno, &ch, 1);
+               n = ops->put_chars(vtno, &ch, 1);
        } while (n <= 0);
 }
 #endif
diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
index 0557f24..853b1d7 100644
--- a/kernel/debug/debug_core.c
+++ b/kernel/debug/debug_core.c
@@ -579,12 +579,14 @@ return_normal:
                kgdb_roundup_cpus(flags);
 #endif

+#ifndef CONFIG_XEN
        /*
         * Wait for the other CPUs to be notified and be waiting for us:
         */
        while (kgdb_do_roundup && (atomic_read(&masters_in_kgdb) +
                                atomic_read(&slaves_in_kgdb)) != online_cpus)
                cpu_relax();
+#endif

        /*
         * At this point the primary processor is completely
--
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/