Messages in this thread | | | From | Andrew Lutomirski <> | Date | Mon, 11 Jul 2011 18:20:50 -0400 | Subject | Re: [PATCH v2 1/8] x86-64: Improve vsyscall emulation CS and RIP handling |
| |
On Mon, Jul 11, 2011 at 6:14 PM, Borislav Petkov <bp@alien8.de> wrote: > On Sat, Jul 09, 2011 at 11:22:08PM -0400, Andy Lutomirski wrote: >> Two fixes here: >> - Send SIGSEGV if called from compat code or with a funny CS. >> - Don't BUG on impossible addresses. >> >> This patch also removes an unused variable. >> >> Signed-off-by: Andy Lutomirski <luto@mit.edu> >> --- >> arch/x86/include/asm/vsyscall.h | 12 -------- >> arch/x86/kernel/vsyscall_64.c | 59 +++++++++++++++++++++++++-------------- >> 2 files changed, 38 insertions(+), 33 deletions(-) >> >> diff --git a/arch/x86/include/asm/vsyscall.h b/arch/x86/include/asm/vsyscall.h >> index bb710cb..d555973 100644 >> --- a/arch/x86/include/asm/vsyscall.h >> +++ b/arch/x86/include/asm/vsyscall.h >> @@ -31,18 +31,6 @@ extern struct timezone sys_tz; >> >> extern void map_vsyscall(void); >> >> -/* Emulation */ >> - >> -static inline bool is_vsyscall_entry(unsigned long addr) >> -{ >> - return (addr & ~0xC00UL) == VSYSCALL_START; >> -} >> - >> -static inline int vsyscall_entry_nr(unsigned long addr) >> -{ >> - return (addr & 0xC00UL) >> 10; >> -} >> - >> #endif /* __KERNEL__ */ >> >> #endif /* _ASM_X86_VSYSCALL_H */ >> diff --git a/arch/x86/kernel/vsyscall_64.c b/arch/x86/kernel/vsyscall_64.c >> index 10cd8ac..6d14848 100644 >> --- a/arch/x86/kernel/vsyscall_64.c >> +++ b/arch/x86/kernel/vsyscall_64.c >> @@ -38,6 +38,7 @@ >> >> #include <asm/vsyscall.h> >> #include <asm/pgtable.h> >> +#include <asm/compat.h> >> #include <asm/page.h> >> #include <asm/unistd.h> >> #include <asm/fixmap.h> >> @@ -97,33 +98,60 @@ static void warn_bad_vsyscall(const char *level, struct pt_regs *regs, >> >> tsk = current; >> >> - printk("%s%s[%d] %s ip:%lx sp:%lx ax:%lx si:%lx di:%lx\n", >> + printk("%s%s[%d] %s ip:%lx cs:%lx sp:%lx ax:%lx si:%lx di:%lx\n", >> level, tsk->comm, task_pid_nr(tsk), >> - message, regs->ip - 2, regs->sp, regs->ax, regs->si, regs->di); >> + message, regs->ip - 2, regs->cs, >> + regs->sp, regs->ax, regs->si, regs->di); >> +} >> + >> +static int addr_to_vsyscall_nr(unsigned *vsyscall_nr, unsigned long addr) >> +{ >> + if ((addr & ~0xC00UL) != VSYSCALL_START) >> + return -EINVAL; >> + >> + *vsyscall_nr = (addr & 0xC00UL) >> 10; >> + if (*vsyscall_nr >= 3) >> + return -EINVAL; >> + >> + return 0; >> } > > I'm wondering: why don't you make this function return negative value on > error, i.e. -EINVAL and the vsyscall number on success so that you can > get rid of returning it through the arg pointer? > > Then at the callsite you can do: > > vsyscall_nr = addr_to_vsyscall_nr(addr); > if (vsyscall_nr < 0) > warn_bad_vsyscall(...)
Because I don't want a warning about ret being used without being initialized.
With the code in this patch, the compiler is smart enough to figure out that either vsyscall_nr is 0, 1, or 2 or that the EINVAL branch is taken. I'll see if it works the other way.
--Andy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |