On Fri, Jun 26, 2020 at 11:14:19AM +0200, Borislav Petkov wrote: > On Thu, Jun 18, 2020 at 01:08:33AM +0300, Jarkko Sakkinen wrote: > > diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst > > b/Documentation/userspace-api/ioctl/ioctl-number.rst > > index 59472cd6a11d..35f713e3a267 100644 > > --- a/Documentation/userspace-api/ioctl/ioctl-number.rst > > +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst > > @@ -323,6 +323,7 @@ Code Seq# Include File > > Comments > > > > <mailto:tle...@mindspring.com> > > 0xA3 90-9F linux/dtlk.h > > 0xA4 00-1F uapi/linux/tee.h > > Generic TEE subsystem > > +0xA4 00-1F uapi/asm/sgx.h Intel > > SGX subsystem (a legit conflict as TEE and SGX do not co-exist) > > Maybe add <mailto:linux-...@vger.kernel.org> ? > > > 0xAA 00-3F linux/uapi/linux/userfaultfd.h > > 0xAB 00-1F linux/nbd.h > > 0xAC 00-1F linux/raw.h > > ... > > > +static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) > > +{ > > + unsigned long encl_size = secs->size + PAGE_SIZE; > > Wait, you just copied @secs from user memory in sgx_ioc_enclave_create() > and now use ->size unverified? You're kidding, right?
The validation is done in sgx_validate_secs(). > > > + struct sgx_epc_page *secs_epc; > > + unsigned long ssaframesize; > > + struct sgx_pageinfo pginfo; > > + struct sgx_secinfo secinfo; > > + struct file *backing; > > + long ret; > > + > > + if (atomic_read(&encl->flags) & SGX_ENCL_CREATED) > > + return -EINVAL; > > + > > + ssaframesize = sgx_calc_ssaframesize(secs->miscselect, secs->xfrm); > > So this is using more un-validated user input to do further calculations. > What can possibly go wrong? > > I sure hope *I* am wrong and am missing something here. > > If not, please, for the next version, audit all your user input and > validate it before using it. Srsly. It works but is unclean. I'd guess reason for this is just that code has evolved into this state over time. I'd just move the call to sgx_calc_ssaframesize() inside sgx_validate_secs(). /Jarkko
