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
| ||
|
Date: Thu, 18 Sep 2014 00:40:29 +0000 From: "Ren, Qiaowei" <qiaowei.ren@...el.com> To: Kevin Easton <kevin@...rana.org> CC: "H. Peter Anvin" <hpa@...or.com>, Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, "Hansen, Dave" <dave.hansen@...el.com>, "x86@...nel.org" <x86@...nel.org>, "linux-mm@...ck.org" <linux-mm@...ck.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org> Subject: RE: [PATCH v8 08/10] x86, mpx: add prctl commands PR_MPX_REGISTER, PR_MPX_UNREGISTER On 2014-09-16, Kevin Easton wrote: > On Thu, Sep 11, 2014 at 04:46:48PM +0800, Qiaowei Ren wrote: >> + >> +int mpx_register(struct task_struct *tsk) { >> + struct mm_struct *mm = tsk->mm; >> + >> + if (!cpu_has_mpx) >> + return -EINVAL; >> + >> + /* >> + * runtime in the userspace will be responsible for allocation of >> + * the bounds directory. Then, it will save the base of the bounds >> + * directory into XSAVE/XRSTOR Save Area and enable MPX through >> + * XRSTOR instruction. >> + * >> + * fpu_xsave() is expected to be very expensive. In order to do >> + * performance optimization, here we get the base of the bounds >> + * directory and then save it into mm_struct to be used in future. >> + */ >> + mm->bd_addr = task_get_bounds_dir(tsk); >> + if (!mm->bd_addr) >> + return -EINVAL; >> + >> + return 0; >> +} >> + >> +int mpx_unregister(struct task_struct *tsk) { >> + struct mm_struct *mm = current->mm; >> + >> + if (!cpu_has_mpx) >> + return -EINVAL; >> + >> + mm->bd_addr = NULL; >> + return 0; >> +} > > If that's changed, then mpx_register() and mpx_unregister() don't need > a task_struct, just an mm_struct. > Yes. An mm_struct is enough. > Probably these functions should be locking mmap_sem. > > Would it be prudent to use an error code other than EINVAL for the > "hardware doesn't support it" case? > Seems like no specific error code for this case. >> @@ -2011,6 +2017,12 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned > long, arg2, unsigned long, arg3, >> me->mm->def_flags &= ~VM_NOHUGEPAGE; >> up_write(&me->mm->mmap_sem); >> break; >> + case PR_MPX_REGISTER: >> + error = MPX_REGISTER(me); >> + break; >> + case PR_MPX_UNREGISTER: >> + error = MPX_UNREGISTER(me); >> + break; > > If you pass me->mm from prctl, that makes it clear that it's > per-process not per-thread, just like PR_SET_DUMPABLE / PR_GET_DUMPABLE. > > This code should also enforce nulls in arg2 / arg3 / arg4,/ arg5 if > it's not using them, otherwise you'll be sunk if you ever want to use them later. > > It seems like it only makes sense for all threads using the mm to have > the same bounds directory set. If the interface was changed to > directly pass the address, then could the kernel take care of setting > it for *all* of the threads in the process? This seems like something > that would be easier for the kernel to do than userspace. > If the interface was changed to this, it will be possible for insane application to pass error bounds directory address to kernel. We still have to call fpu_xsave() to check this. Thanks, Qiaowei -- 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/
Powered by blists - more mailing lists