Messages in this thread | | | Date | Tue, 21 Nov 2017 08:13:21 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH v3] x86/umip: Warn if UMIP-protected instructions are used |
| |
* Ricardo Neri <ricardo.neri-calderon@linux.intel.com> wrote:
> Print a rate-limited warning when a user space program attempts to execute > any of the instructions that UMIP protects (i.e., SGDT, SIDT, SLDT, STR > and SMSW). > > This is useful because, when CONFIG_X86_INTEL_UMIP is selected and > supported by the hardware, user space programs that try to execute such > instructions will receive a SIGSEGV signal that they might not expect. > In the specific cases for which emulation is provided (instructions SGDT, > SIDT and SMSW in protected and virtual-8086 modes), no signal is > generated. However, a warning is helpful to encourage updates in such > programs to avoid the use of such instructions. > > Warnings are printed via a customized printk() function that also provides > information about the program that attempted to use the affected > instructions. > > Utility macros are defined to wrap umip_printk() for the error and warning > kernel log levels. > > While here, replace an existing call to the generic rate-limited pr_err() > with the new umip_pr_err(). > > Cc: Andy Lutomirski <luto@kernel.org> > Cc: H. Peter Anvin <hpa@zytor.com> > Cc: Borislav Petkov <bp@suse.de> > Cc: Tony Luck <tony.luck@intel.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Ravi V. Shankar <ravi.v.shankar@intel.com> > Cc: x86@kernel.org > Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> > --- > Changes since V2: > * Dropped patches 1, 2, and 3 as they were applied on the tip repository. > * Renamed umip_pr_warn() to umip_printk(). Instead, added utility macros > for the error and warning kernel log levels. > * Implemented umip_printk() as a variadic function. This also includes > using the __printf() macro to type-check arguments of the format > string and arguments. > * Capitalized all the instructions' mnemonics in strings used for printing > warnings. > > Changes since V1: > * Capitalized all the instructions' mnemonics in both code and patch > descriptions. > * Corrected documentation of umip_pr_warn() to correctly reflect the function > name. > * Updated description of patch #4 to describe the update to the existing > rate-limited pr_err(). > --- > arch/x86/kernel/umip.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 58 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kernel/umip.c b/arch/x86/kernel/umip.c > index 1f1f2d5..dabbac3 100644 > --- a/arch/x86/kernel/umip.c > +++ b/arch/x86/kernel/umip.c > @@ -82,6 +82,57 @@ > #define UMIP_INST_SLDT 3 /* 0F 00 /0 */ > #define UMIP_INST_STR 4 /* 0F 00 /1 */ > > +const char * const umip_insns[5] = { > + [UMIP_INST_SGDT] = "SGDT", > + [UMIP_INST_SIDT] = "SIDT", > + [UMIP_INST_SMSW] = "SMSW", > + [UMIP_INST_SLDT] = "SLDT", > + [UMIP_INST_STR] = "STR", > +}; > + > +#define umip_pr_err(regs, fmt, ...) \ > + umip_printk(regs, KERN_ERR, fmt, ##__VA_ARGS__) > +#define umip_pr_warning(regs, fmt, ...) \ > + umip_printk(regs, KERN_WARNING, fmt, ##__VA_ARGS__) > + > +/** > + * umip_printk() - Print a rate-limited message > + * @regs: Register set with the context in which the warning is printed > + * @log_level: Kernel log level to print the message > + * @fmt: The text string to print > + * > + * Print the text contained in @fmt. The print rate is limited to bursts of 5 > + * messages every two minutes. The purpose of this customized version of > + * printk() is to print messages when user space processes use any of the > + * UMIP-protected instructions. Thus, the printed text is prepended with the > + * task name and process ID number of the current task as well as the > + * instruction and stack pointers in @regs as seen when entering kernel mode. > + * > + * Returns: > + * > + * None. > + */ > +static __printf(3, 4) > +void umip_printk(const struct pt_regs *regs, const char *log_level, > + const char *fmt, ...) > +{ > + /* Bursts of 5 messages every two minutes */ > + static DEFINE_RATELIMIT_STATE(ratelimit, 2 * 60 * HZ, 5); > + struct task_struct *tsk = current; > + struct va_format vaf; > + va_list args; > + > + if (!__ratelimit(&ratelimit)) > + return; > + > + va_start(args, fmt); > + vaf.fmt = fmt; > + vaf.va = &args; > + printk("%s" pr_fmt("%s[%d] ip:%lx sp:%lx: %pV"), log_level, tsk->comm, > + task_pid_nr(tsk), regs->ip, regs->sp, &vaf); > + va_end(args); > +} > + > /** > * identify_insn() - Identify a UMIP-protected instruction > * @insn: Instruction structure with opcode and ModRM byte. > @@ -236,10 +287,8 @@ static void force_sig_info_umip_fault(void __user *addr, struct pt_regs *regs) > if (!(show_unhandled_signals && unhandled_signal(tsk, SIGSEGV))) > return; > > - pr_err_ratelimited("%s[%d] umip emulation segfault ip:%lx sp:%lx error:%x in %lx\n", > - tsk->comm, task_pid_nr(tsk), regs->ip, > - regs->sp, X86_PF_USER | X86_PF_WRITE, > - regs->ip); > + umip_pr_err(regs, "segfault in emulation. error%x\n", > + X86_PF_USER | X86_PF_WRITE); > } > > /** > @@ -326,10 +375,15 @@ bool fixup_umip_exception(struct pt_regs *regs) > if (umip_inst < 0) > return false; > > + umip_pr_warning(regs, "%s instruction cannot be used by applications.\n", > + umip_insns[umip_inst]); > + > /* Do not emulate SLDT, STR or user long mode processes. */ > if (umip_inst == UMIP_INST_STR || umip_inst == UMIP_INST_SLDT || user_64bit_mode(regs)) > return false; > > + umip_pr_warning(regs, "For now, expensive software emulation returns the result.\n");
Ok, this looks much nicer all around. I'll test it and push it out if all is good.
Thanks,
Ingo
| |