| Message ID | 20190819152544.7296-4-jarkko.sakkinen@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/sgx: Improve permission handing | expand |
On Mon, Aug 19, 2019 at 06:25:42PM +0300, Jarkko Sakkinen wrote: > Split the huge conditional statement to three separate ones in > order to make it easier to understand what is going on in the > validation code. > > Cc: Sean Christopherson <sean.j.christpherson@intel.com> > Cc: Shay Katz-zamir <shay.katz-zamir@intel.com> > Cc: Serge Ayoun <serge.ayoun@intel.com> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > --- > arch/x86/kernel/cpu/sgx/driver/ioctl.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kernel/cpu/sgx/driver/ioctl.c b/arch/x86/kernel/cpu/sgx/driver/ioctl.c > index d5f326411df0..99b1b9776c3a 100644 > --- a/arch/x86/kernel/cpu/sgx/driver/ioctl.c > +++ b/arch/x86/kernel/cpu/sgx/driver/ioctl.c > @@ -415,10 +415,15 @@ static int sgx_validate_secinfo(struct sgx_secinfo *secinfo) > u64 page_type = secinfo->flags & SGX_SECINFO_PAGE_TYPE_MASK; > u64 perm = secinfo->flags & SGX_SECINFO_PERMISSION_MASK; > > - if ((secinfo->flags & SGX_SECINFO_RESERVED_MASK) || > - ((perm & SGX_SECINFO_W) && !(perm & SGX_SECINFO_R)) || > - (page_type != SGX_SECINFO_TCS && page_type != SGX_SECINFO_TRIM && > - page_type != SGX_SECINFO_REG)) > + if ((page_type != SGX_SECINFO_REG && > + page_type != SGX_SECINFO_TCS && > + page_type != SGX_SECINFO_TRIM)) Shouldn't we disallow TRIM until SGX2 is supported? > + return -EINVAL; > + > + if ((perm & SGX_SECINFO_W) && !(perm & SGX_SECINFO_R)) > + return -EINVAL; > + > + if (secinfo->flags & SGX_SECINFO_RESERVED_MASK) > return -EINVAL; > > if (memchr_inv(secinfo->reserved, 0, SGX_SECINFO_RESERVED_SIZE)) > -- > 2.20.1 >
> From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > Sent: Monday, August 19, 2019 18:26 > To: linux-sgx@vger.kernel.org > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>; Sean Christopherson > <sean.j.christpherson@intel.com>; Katz-zamir, Shay <shay.katz- > zamir@intel.com>; Ayoun, Serge <serge.ayoun@intel.com> > Subject: [PATCH 3/5] x86/sgx: Make sgx_validate_secinfo() more readable > > Split the huge conditional statement to three separate ones in order to make > it easier to understand what is going on in the validation code. > > Cc: Sean Christopherson <sean.j.christpherson@intel.com> > Cc: Shay Katz-zamir <shay.katz-zamir@intel.com> > Cc: Serge Ayoun <serge.ayoun@intel.com> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > --- > arch/x86/kernel/cpu/sgx/driver/ioctl.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kernel/cpu/sgx/driver/ioctl.c > b/arch/x86/kernel/cpu/sgx/driver/ioctl.c > index d5f326411df0..99b1b9776c3a 100644 > --- a/arch/x86/kernel/cpu/sgx/driver/ioctl.c > +++ b/arch/x86/kernel/cpu/sgx/driver/ioctl.c > @@ -415,10 +415,15 @@ static int sgx_validate_secinfo(struct sgx_secinfo > *secinfo) > u64 page_type = secinfo->flags & SGX_SECINFO_PAGE_TYPE_MASK; > u64 perm = secinfo->flags & SGX_SECINFO_PERMISSION_MASK; > > - if ((secinfo->flags & SGX_SECINFO_RESERVED_MASK) || > - ((perm & SGX_SECINFO_W) && !(perm & SGX_SECINFO_R)) || > - (page_type != SGX_SECINFO_TCS && page_type != > SGX_SECINFO_TRIM && > - page_type != SGX_SECINFO_REG)) > + if ((page_type != SGX_SECINFO_REG && > + page_type != SGX_SECINFO_TCS && > + page_type != SGX_SECINFO_TRIM)) > + return -EINVAL; sgx_validate_secinfo() is called via eadd ioctl. Eadd will fail with TRIM page type, so you probably need to remove it from the if sgx2.0 does not change this behavior > + > + if ((perm & SGX_SECINFO_W) && !(perm & SGX_SECINFO_R)) > + return -EINVAL; > + > + if (secinfo->flags & SGX_SECINFO_RESERVED_MASK) > return -EINVAL; > > if (memchr_inv(secinfo->reserved, 0, > SGX_SECINFO_RESERVED_SIZE)) > -- > 2.20.1 --------------------------------------------------------------------- Intel Israel (74) Limited This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
On Wed, 2019-08-21 at 20:48 -0700, Sean Christopherson wrote: > > + if ((page_type != SGX_SECINFO_REG && > > + page_type != SGX_SECINFO_TCS && > > + page_type != SGX_SECINFO_TRIM)) > > Shouldn't we disallow TRIM until SGX2 is supported? Yes. I squashed the change with TRIM removed. I also renamed the local variable page_type as pt. I think that is a good abbrevation for page type and saves some screen space. /Jarkko
On Thu, 2019-08-22 at 10:39 +0000, Ayoun, Serge wrote: > > + if ((page_type != SGX_SECINFO_REG && > > + page_type != SGX_SECINFO_TCS && > > + page_type != SGX_SECINFO_TRIM)) > > + return -EINVAL; > > sgx_validate_secinfo() is called via eadd ioctl. Eadd will fail with > TRIM page type, so you probably need to remove it from the if > sgx2.0 does not change this behavior Thanks! Removed. /Jarkko
diff --git a/arch/x86/kernel/cpu/sgx/driver/ioctl.c b/arch/x86/kernel/cpu/sgx/driver/ioctl.c index d5f326411df0..99b1b9776c3a 100644 --- a/arch/x86/kernel/cpu/sgx/driver/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/driver/ioctl.c @@ -415,10 +415,15 @@ static int sgx_validate_secinfo(struct sgx_secinfo *secinfo) u64 page_type = secinfo->flags & SGX_SECINFO_PAGE_TYPE_MASK; u64 perm = secinfo->flags & SGX_SECINFO_PERMISSION_MASK; - if ((secinfo->flags & SGX_SECINFO_RESERVED_MASK) || - ((perm & SGX_SECINFO_W) && !(perm & SGX_SECINFO_R)) || - (page_type != SGX_SECINFO_TCS && page_type != SGX_SECINFO_TRIM && - page_type != SGX_SECINFO_REG)) + if ((page_type != SGX_SECINFO_REG && + page_type != SGX_SECINFO_TCS && + page_type != SGX_SECINFO_TRIM)) + return -EINVAL; + + if ((perm & SGX_SECINFO_W) && !(perm & SGX_SECINFO_R)) + return -EINVAL; + + if (secinfo->flags & SGX_SECINFO_RESERVED_MASK) return -EINVAL; if (memchr_inv(secinfo->reserved, 0, SGX_SECINFO_RESERVED_SIZE))
Split the huge conditional statement to three separate ones in order to make it easier to understand what is going on in the validation code. Cc: Sean Christopherson <sean.j.christpherson@intel.com> Cc: Shay Katz-zamir <shay.katz-zamir@intel.com> Cc: Serge Ayoun <serge.ayoun@intel.com> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> --- arch/x86/kernel/cpu/sgx/driver/ioctl.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)