| Message ID | 20190613164240.31326-1-jarkko.sakkinen@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] x86/sgx: Check that the address is within ELRANGE | expand |
On Thu, Jun 13, 2019 at 07:42:40PM +0300, Jarkko Sakkinen wrote: > In sgx_encl_page_alloc() check that the given address within the > ELRANGE. Return -EINVAL if not. > > Reported-by: Shay Katz-zamir <shay.katz-zamir@intel.com> > Cc: Sean Christopherson <sean.j.christopherson@intel.com> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > --- > arch/x86/kernel/cpu/sgx/driver/ioctl.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/x86/kernel/cpu/sgx/driver/ioctl.c b/arch/x86/kernel/cpu/sgx/driver/ioctl.c > index d17c60dca114..9d3fc770b4d9 100644 > --- a/arch/x86/kernel/cpu/sgx/driver/ioctl.c > +++ b/arch/x86/kernel/cpu/sgx/driver/ioctl.c > @@ -240,19 +240,26 @@ static struct sgx_encl_page *sgx_encl_page_alloc(struct sgx_encl *encl, > struct sgx_encl_page *encl_page; > int ret; > > + if (addr < encl->base || addr > (encl->base + encl->size)) The upper bounds check should be "addr >= (encl->base + encl->size)" or "addr > (encl->base + encl->size - 1)" > + return ERR_PTR(-EINVAL); > + > if (radix_tree_lookup(&encl->page_tree, PFN_DOWN(addr))) > return ERR_PTR(-EEXIST); > + > encl_page = kzalloc(sizeof(*encl_page), GFP_KERNEL); > if (!encl_page) > return ERR_PTR(-ENOMEM); > + > encl_page->desc = addr; > encl_page->encl = encl; > + > ret = radix_tree_insert(&encl->page_tree, PFN_DOWN(encl_page->desc), > encl_page); > if (ret) { > kfree(encl_page); > return ERR_PTR(ret); > } > + > return encl_page; > } > > -- > 2.20.1 >
On Mon, 2019-06-17 at 15:30 -0700, Sean Christopherson wrote: > On Thu, Jun 13, 2019 at 07:42:40PM +0300, Jarkko Sakkinen wrote: > > In sgx_encl_page_alloc() check that the given address within the > > ELRANGE. Return -EINVAL if not. > > > > Reported-by: Shay Katz-zamir <shay.katz-zamir@intel.com> > > Cc: Sean Christopherson <sean.j.christopherson@intel.com> > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > --- > > arch/x86/kernel/cpu/sgx/driver/ioctl.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/arch/x86/kernel/cpu/sgx/driver/ioctl.c > > b/arch/x86/kernel/cpu/sgx/driver/ioctl.c > > index d17c60dca114..9d3fc770b4d9 100644 > > --- a/arch/x86/kernel/cpu/sgx/driver/ioctl.c > > +++ b/arch/x86/kernel/cpu/sgx/driver/ioctl.c > > @@ -240,19 +240,26 @@ static struct sgx_encl_page > > *sgx_encl_page_alloc(struct sgx_encl *encl, > > struct sgx_encl_page *encl_page; > > int ret; > > > > + if (addr < encl->base || addr > (encl->base + encl->size)) > > The upper bounds check should be "addr >= (encl->base + encl->size)" or > "addr > (encl->base + encl->size - 1)" Right, thanks :-) Shay told me that this did not happen with earlier patch set versions but I could not spot why. Have to look again before merging. /Jarkko
diff --git a/arch/x86/kernel/cpu/sgx/driver/ioctl.c b/arch/x86/kernel/cpu/sgx/driver/ioctl.c index d17c60dca114..9d3fc770b4d9 100644 --- a/arch/x86/kernel/cpu/sgx/driver/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/driver/ioctl.c @@ -240,19 +240,26 @@ static struct sgx_encl_page *sgx_encl_page_alloc(struct sgx_encl *encl, struct sgx_encl_page *encl_page; int ret; + if (addr < encl->base || addr > (encl->base + encl->size)) + return ERR_PTR(-EINVAL); + if (radix_tree_lookup(&encl->page_tree, PFN_DOWN(addr))) return ERR_PTR(-EEXIST); + encl_page = kzalloc(sizeof(*encl_page), GFP_KERNEL); if (!encl_page) return ERR_PTR(-ENOMEM); + encl_page->desc = addr; encl_page->encl = encl; + ret = radix_tree_insert(&encl->page_tree, PFN_DOWN(encl_page->desc), encl_page); if (ret) { kfree(encl_page); return ERR_PTR(ret); } + return encl_page; }
In sgx_encl_page_alloc() check that the given address within the ELRANGE. Return -EINVAL if not. Reported-by: Shay Katz-zamir <shay.katz-zamir@intel.com> Cc: Sean Christopherson <sean.j.christopherson@intel.com> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> --- arch/x86/kernel/cpu/sgx/driver/ioctl.c | 7 +++++++ 1 file changed, 7 insertions(+)