linux-kernel - Re: [RFC] mmiotrace full patch, preview 2

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: <20080328202435.6a58e21d@daedalus.pq.iki.fi>
Date:	Fri, 28 Mar 2008 20:24:35 +0200
From:	Pekka Paalanen <pq@....fi>
To:	"Vegard Nossum" <vegard.nossum@...il.com>
Cc:	linux-kernel@...r.kernel.org, "Ingo Molnar" <mingo@...e.hu>,
	"Christoph Hellwig" <hch@...radead.org>,
	"Arjan van de Ven" <arjan@...radead.org>,
	"Pavel Roskin" <proski@....org>,
	"Steven Rostedt" <rostedt@...dmis.org>,
	"Peter Zijlstra" <a.p.zijlstra@...llo.nl>
Subject: Re: [RFC] mmiotrace full patch, preview 2

On Fri, 28 Mar 2008 00:13:48 +0100
"Vegard Nossum" <vegard.nossum@...il.com> wrote:

> I may of course be wrong, but... Shouldn't the post_kmmio_handler(),
> called from the die notifier chain, check for the DR_STEP condition?
> This makes sure that the function is not called in the cases where the
> source of the debug exception was not a single-stepping event. Though
> I guess you'll also have other checks in place to notice that the
> interrupt was not the one you were expecting. I guess a little extra
> safety won't hurt though?
> 
> On Sun, Mar 9, 2008 at 3:40 PM, Pekka Paalanen <pq@....fi> wrote:
> >  +/*
> >  + * Interrupts are disabled on entry as trap1 is an interrupt gate
> >  + * and they remain disabled thorough out this function.
> >  + * This must always get called as the pair to kmmio_handler().
> >  + */
> >  +static int post_kmmio_handler(unsigned long condition, struct pt_regs *regs)
> >  +{
> >  +       int ret = 0;
> >  +       struct kmmio_context *ctx = &get_cpu_var(kmmio_ctx);
> 
> if (!(condition & DR_STEP))
>     return;

I guess that would be appropriate. I think it should go into this
function:

+static int kmmio_die_notifier(struct notifier_block *nb, unsigned long val,
+								void *args)
+{
+	struct die_args *arg = args;
+
+	if (val == DIE_DEBUG)
+		if (post_kmmio_handler(arg->err, arg->regs) == 1)
+			return NOTIFY_STOP;
+
+	return NOTIFY_DONE;
+}

On the other hand I am thinking of not using the die notifier chain
at all and adding a direct call from do_debug() or something.
This is the last dynamic hook remaining from the out-of-tree module
era of mmiotrace.

I guess with the notifier list there is a possibility that another
module intercepts my single step trap, so that this is never called,
which would leave mmiotrace half blind, and also trigger a recursive
probe hit.

btw. what if someone uses kmemcheck and mmiotrace at the same time?
Mmiotrace will not fiddle with any other pages than returned via
__ioremap(), but can kmemcheck "hide" the same mmio pages?
Also keeping in mind, that some day I'd like to make mmiotrace able
to catch mmio accesses originating in user space.


Thanks.

-- 
Pekka Paalanen
http://www.iki.fi/pq/
--
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/