linux-kernel - Re: [PATCH v13 3/3] trace,x86: Add irq vector tracepoints

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 for Android: free password hash cracker in your pocket

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

Message-ID: <1370303581.26799.109.camel@gandalf.local.home>
Date:	Mon, 03 Jun 2013 19:53:01 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Seiji Aguchi <seiji.aguchi@....com>
Cc:	linux-kernel@...r.kernel.org, x86@...nel.org, hpa@...or.com,
	tglx@...utronix.de, mingo@...e.hu, bp@...en8.de,
	linux-edac@...r.kernel.org, tony.luck@...el.com,
	dle-develop@...ts.sourceforge.net, tomoki.sekiyama@....com
Subject: Re: [PATCH v13 3/3] trace,x86: Add irq vector tracepoints

On Mon, 2013-06-03 at 15:29 -0400, Seiji Aguchi wrote:

Yeah, I believe this does work. But you probably should add a comment
like the following:

/*
 * The current_idt_descr_ptr can only be set out of interrupt context
 * to avoid races. Once set, the load_current_idt() is called by
interrupt
 * context either by NMI, debug, or via a smp_call_function(). That way
 * the IDT will always be set back to the expected descriptor.
 */
>  
> +extern atomic_long_t current_idt_descr_ptr;
> +static inline void load_current_idt(void)
> +{
> +	if (atomic_long_read(&current_idt_descr_ptr))

Also, we should probably add here:
		unsigned long new_idt = atomic_long_read(&current_idt_descr_ptr);

		if (WARN_ON_ONCE(!validate_idt(new_idt))
			return;
		load_idt((const struct desc_ptr *)new_idt);

> +		load_idt((const struct desc_ptr *)
> +			 atomic_long_read(&current_idt_descr_ptr));
> +	else
> +		load_idt((const struct desc_ptr *)&idt_descr);
> +}
> +

Then have 

bool validate_idt(unsigned long idt)
{
	switch(idt) {
	case (unsigned long)&trace_idt_descr:
	case (unsigned long)&idt_descr:
		return 0;
	}
	return -1;
}

This way we wont be opening up any easy root holes where if a process
finds a way to modify some arbitrary kernel memory, we can prevent it
from modifying the current_idt_descr_ptr and have a nice way to exploit
the IDT. Sure, one can argue that if they can modify arbitrary kernel
memory, we may already be lost, but lets not make it easier for them
than need be.

-- Steve


--
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/