linux-kernel - Re: [PATCH] x86: Use v8086_mode helper, trivial unification

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: <1200618952.5724.85.camel@brick>
Date:	Thu, 17 Jan 2008 17:15:52 -0800
From:	Harvey Harrison <harvey.harrison@...il.com>
To:	"H. Peter Anvin" <hpa@...or.com>
Cc:	Ingo Molnar <mingo@...e.hu>, LKML <linux-kernel@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Roland McGrath <roland@...hat.com>
Subject: Re: [PATCH] x86: Use v8086_mode helper, trivial unification

Roland actually put on CC this time.

On Thu, 2008-01-17 at 19:59 -0500, H. Peter Anvin wrote:
> Harvey Harrison wrote:
> > 
> > Sorry, missed that detail in ptrace.h, I notice now.
> > 
> > Is there some better way this could be organized, would the following
> > be an improvement, as opposed to two long ifdef sections?
> > 
> > Patch will follow if you think it's a good idea.
> 
> It is actually quite a bit easier to read.

I'll send along a patch along soon, any thoughts on how to order it in
the file?

> 
> > 
> > static inline unsigned long stack_pointer(struct pt_regs *regs)
> > {
> > #ifdef CONFIG_X86_32
> > 	return (unsigned long)regs;
> > #else
> > 	return regs->sp;
> > #endif
> > }
> 
> This one is kind of strange.  In particular, the 32-bit definition isn't 
> exactly what one would expect.  It makes me concerned that it actually 
> refers to two different kinds of stack pointers?

This tripped up the kprobes unification as well, see the stack_addr()
helper that was introduced there.  Would be good to figure this out
and put a big fat comment on it.

kprobes.c

#ifdef CONFIG_X86_64
#define stack_addr(regs) ((unsigned long *)regs->sp)
#else
/*
 * "&regs->sp" looks wrong, but it's correct for x86_32.  x86_32 CPUs
 * don't save the ss and esp registers if the CPU is already in kernel
 * mode when it traps.  So for kprobes, regs->sp and regs->ss are not
 * the [nonexistent] saved stack pointer and ss register, but rather
 * the top 8 bytes of the pre-int3 stack.  So &regs->sp happens to
 * point to the top of the pre-int3 stack.
 */
#define stack_addr(regs) ((unsigned long *)&regs->sp)
#endif

> > /* still need a define here, as one is long and one is unsigned long.
> >  * but this is another target for unification I guess. */
> > #define regs_return_value(regs) ((regs)->ax)
> 
> Indeed...

I think this comes out of Roland's patches unifying some names eip/rip,
eax/rax, etc.

CC'd in case he felt like more work ;-)

Harvey

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