diff mbox series

[RFC,v3,1/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup

Message ID 20240515065521.67908-2-zhubojun.zbj@antgroup.com (mailing list archive)
State New, archived
Headers show
Series x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup | expand

Commit Message

Bojun Zhu May 15, 2024, 6:55 a.m. UTC
EDMM's ioctl()s support batch operations, which may be
time-consuming. Try to explicitly give up the CPU as the prefix
operation at the every begin of "for loop" in
sgx_enclave_{ modify_types | restrict_permissions | remove_pages}
to give other tasks a chance to run, and avoid softlockup warning.

Additionally perform pending signals check as the prefix operation,
and introduce sgx_check_signal_and_resched(),
which wraps all the checks.

The following has been observed on Linux v6.9-rc5 with kernel
preemptions disabled(by configuring "PREEMPT_NONE=y"), when kernel
is requested to restrict page permissions of a large number of EPC pages.

    ------------[ cut here ]------------
    watchdog: BUG: soft lockup - CPU#45 stuck for 22s!
    ...
    RIP: 0010:sgx_enclave_restrict_permissions+0xba/0x1f0
    ...
    Call Trace:
     sgx_ioctl
     __x64_sys_ioctl
     x64_sys_call
     do_syscall_64
     entry_SYSCALL_64_after_hwframe
    ------------[ end trace ]------------

Signed-off-by: Bojun Zhu <zhubojun.zbj@antgroup.com>
---
 arch/x86/kernel/cpu/sgx/ioctl.c | 40 +++++++++++++++++++++++----------
 1 file changed, 28 insertions(+), 12 deletions(-)

Comments

Jarkko Sakkinen May 15, 2024, 12:06 p.m. UTC | #1
On Wed May 15, 2024 at 9:55 AM EEST, Bojun Zhu wrote:
> EDMM's ioctl()s support batch operations, which may be
> time-consuming. Try to explicitly give up the CPU as the prefix
> operation at the every begin of "for loop" in
> sgx_enclave_{ modify_types | restrict_permissions | remove_pages}
> to give other tasks a chance to run, and avoid softlockup warning.
>
> Additionally perform pending signals check as the prefix operation,
> and introduce sgx_check_signal_and_resched(),
> which wraps all the checks.
>
> The following has been observed on Linux v6.9-rc5 with kernel
> preemptions disabled(by configuring "PREEMPT_NONE=y"), when kernel
> is requested to restrict page permissions of a large number of EPC pages.
>
>     ------------[ cut here ]------------
>     watchdog: BUG: soft lockup - CPU#45 stuck for 22s!
>     ...
>     RIP: 0010:sgx_enclave_restrict_permissions+0xba/0x1f0
>     ...
>     Call Trace:
>      sgx_ioctl
>      __x64_sys_ioctl
>      x64_sys_call
>      do_syscall_64
>      entry_SYSCALL_64_after_hwframe
>     ------------[ end trace ]------------
>

Suggested-by: Jarkko Sakkinen <jarkko@kernel.org>

> Signed-off-by: Bojun Zhu <zhubojun.zbj@antgroup.com>
> ---
>  arch/x86/kernel/cpu/sgx/ioctl.c | 40 +++++++++++++++++++++++----------
>  1 file changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
> index b65ab214bdf5..6199f483143e 100644
> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> @@ -365,6 +365,20 @@ static int sgx_validate_offset_length(struct sgx_encl *encl,
>  	return 0;
>  }
>  
> +/*
> + * Check signals and invoke scheduler. Return true for a pending signal.
> + */
> +static bool sgx_check_signal_and_resched(void)
> +{
> +	if (signal_pending(current))
> +		return true;
> +
> +	if (need_resched())
> +		cond_resched();
> +
> +	return false;
> +}
> +
>  /**
>   * sgx_ioc_enclave_add_pages() - The handler for %SGX_IOC_ENCLAVE_ADD_PAGES
>   * @encl:       an enclave pointer
> @@ -409,7 +423,7 @@ static long sgx_ioc_enclave_add_pages(struct sgx_encl *encl, void __user *arg)
>  	struct sgx_enclave_add_pages add_arg;
>  	struct sgx_secinfo secinfo;
>  	unsigned long c;
> -	int ret;
> +	int ret = -ERESTARTSYS;
>  
>  	if (!test_bit(SGX_ENCL_CREATED, &encl->flags) ||
>  	    test_bit(SGX_ENCL_INITIALIZED, &encl->flags))
> @@ -432,15 +446,8 @@ static long sgx_ioc_enclave_add_pages(struct sgx_encl *encl, void __user *arg)
>  		return -EINVAL;
>  
>  	for (c = 0 ; c < add_arg.length; c += PAGE_SIZE) {
> -		if (signal_pending(current)) {
> -			if (!c)
> -				ret = -ERESTARTSYS;
> -
> +		if (sgx_check_signal_and_resched())
>  			break;
> -		}
> -
> -		if (need_resched())
> -			cond_resched();
>  
>  		ret = sgx_encl_add_page(encl, add_arg.src + c, add_arg.offset + c,
>  					&secinfo, add_arg.flags);
> @@ -740,12 +747,15 @@ sgx_enclave_restrict_permissions(struct sgx_encl *encl,
>  	unsigned long addr;
>  	unsigned long c;
>  	void *epc_virt;
> -	int ret;
> +	int ret = -ERESTARTSYS;
>  
>  	memset(&secinfo, 0, sizeof(secinfo));
>  	secinfo.flags = modp->permissions & SGX_SECINFO_PERMISSION_MASK;
>  
>  	for (c = 0 ; c < modp->length; c += PAGE_SIZE) {
> +		if (sgx_check_signal_and_resched())
> +			goto out;
> +
>  		addr = encl->base + modp->offset + c;
>  
>  		sgx_reclaim_direct();
> @@ -898,7 +908,7 @@ static long sgx_enclave_modify_types(struct sgx_encl *encl,
>  	unsigned long addr;
>  	unsigned long c;
>  	void *epc_virt;
> -	int ret;
> +	int ret = -ERESTARTSYS;
>  
>  	page_type = modt->page_type & SGX_PAGE_TYPE_MASK;
>  
> @@ -913,6 +923,9 @@ static long sgx_enclave_modify_types(struct sgx_encl *encl,
>  	secinfo.flags = page_type << 8;
>  
>  	for (c = 0 ; c < modt->length; c += PAGE_SIZE) {
> +		if (sgx_check_signal_and_resched())
> +			goto out;
> +
>  		addr = encl->base + modt->offset + c;
>  
>  		sgx_reclaim_direct();
> @@ -1095,12 +1108,15 @@ static long sgx_encl_remove_pages(struct sgx_encl *encl,
>  	unsigned long addr;
>  	unsigned long c;
>  	void *epc_virt;
> -	int ret;
> +	int ret = -ERESTARTSYS;
>  
>  	memset(&secinfo, 0, sizeof(secinfo));
>  	secinfo.flags = SGX_SECINFO_R | SGX_SECINFO_W | SGX_SECINFO_X;
>  
>  	for (c = 0 ; c < params->length; c += PAGE_SIZE) {
> +		if (sgx_check_signal_and_resched())
> +			goto out;
> +
>  		addr = encl->base + params->offset + c;
>  
>  		sgx_reclaim_direct();

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko
Haitao Huang May 15, 2024, 9:55 p.m. UTC | #2
On Wed, 15 May 2024 01:55:21 -0500, Bojun Zhu <zhubojun.zbj@antgroup.com>  
wrote:

> EDMM's ioctl()s support batch operations, which may be
> time-consuming. Try to explicitly give up the CPU as the prefix
> operation at the every begin of "for loop" in
> sgx_enclave_{ modify_types | restrict_permissions | remove_pages}
> to give other tasks a chance to run, and avoid softlockup warning.
>
> Additionally perform pending signals check as the prefix operation,
> and introduce sgx_check_signal_and_resched(),
> which wraps all the checks.
>
> The following has been observed on Linux v6.9-rc5 with kernel
> preemptions disabled(by configuring "PREEMPT_NONE=y"), when kernel
> is requested to restrict page permissions of a large number of EPC pages.
>
>     ------------[ cut here ]------------
>     watchdog: BUG: soft lockup - CPU#45 stuck for 22s!
>     ...
>     RIP: 0010:sgx_enclave_restrict_permissions+0xba/0x1f0
>     ...
>     Call Trace:
>      sgx_ioctl
>      __x64_sys_ioctl
>      x64_sys_call
>      do_syscall_64
>      entry_SYSCALL_64_after_hwframe
>     ------------[ end trace ]------------
>
> Signed-off-by: Bojun Zhu <zhubojun.zbj@antgroup.com>
> ---
>  arch/x86/kernel/cpu/sgx/ioctl.c | 40 +++++++++++++++++++++++----------
>  1 file changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c  
> b/arch/x86/kernel/cpu/sgx/ioctl.c
> index b65ab214bdf5..6199f483143e 100644
> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> @@ -365,6 +365,20 @@ static int sgx_validate_offset_length(struct  
> sgx_encl *encl,
>  	return 0;
>  }
> +/*
> + * Check signals and invoke scheduler. Return true for a pending signal.
> + */
> +static bool sgx_check_signal_and_resched(void)
> +{
> +	if (signal_pending(current))
> +		return true;
> +
> +	if (need_resched())
> +		cond_resched();
> +
> +	return false;
> +}
> +
>  /**
>   * sgx_ioc_enclave_add_pages() - The handler for  
> %SGX_IOC_ENCLAVE_ADD_PAGES
>   * @encl:       an enclave pointer
> @@ -409,7 +423,7 @@ static long sgx_ioc_enclave_add_pages(struct  
> sgx_encl *encl, void __user *arg)
>  	struct sgx_enclave_add_pages add_arg;
>  	struct sgx_secinfo secinfo;
>  	unsigned long c;
> -	int ret;
> +	int ret = -ERESTARTSYS;
> 	if (!test_bit(SGX_ENCL_CREATED, &encl->flags) ||
>  	    test_bit(SGX_ENCL_INITIALIZED, &encl->flags))
> @@ -432,15 +446,8 @@ static long sgx_ioc_enclave_add_pages(struct  
> sgx_encl *encl, void __user *arg)
>  		return -EINVAL;
> 	for (c = 0 ; c < add_arg.length; c += PAGE_SIZE) {
> -		if (signal_pending(current)) {
> -			if (!c)
> -				ret = -ERESTARTSYS;
> -
> +		if (sgx_check_signal_and_resched())
>  			break;
> -		}

ERESTARTSYS is only appropriate if we have not EADDed any pages yet.
If we got interrupted in the middle, we should return 0. User space would  
check the 'count' returned and decide to recall this ioctl() with  
'offset'  reset to the next page, and adjust length.

Ditto for other changes below.

Thanks
Haitao
Haitao Huang May 15, 2024, 10:29 p.m. UTC | #3
On Wed, 15 May 2024 16:55:59 -0500, Haitao Huang  
<haitao.huang@linux.intel.com> wrote:

> On Wed, 15 May 2024 01:55:21 -0500, Bojun Zhu  
> <zhubojun.zbj@antgroup.com> wrote:
>
>> EDMM's ioctl()s support batch operations, which may be
>> time-consuming. Try to explicitly give up the CPU as the prefix
>> operation at the every begin of "for loop" in
>> sgx_enclave_{ modify_types | restrict_permissions | remove_pages}
>> to give other tasks a chance to run, and avoid softlockup warning.
>>
>> Additionally perform pending signals check as the prefix operation,
>> and introduce sgx_check_signal_and_resched(),
>> which wraps all the checks.
>>
>> The following has been observed on Linux v6.9-rc5 with kernel
>> preemptions disabled(by configuring "PREEMPT_NONE=y"), when kernel
>> is requested to restrict page permissions of a large number of EPC  
>> pages.
>>
>>     ------------[ cut here ]------------
>>     watchdog: BUG: soft lockup - CPU#45 stuck for 22s!
>>     ...
>>     RIP: 0010:sgx_enclave_restrict_permissions+0xba/0x1f0
>>     ...
>>     Call Trace:
>>      sgx_ioctl
>>      __x64_sys_ioctl
>>      x64_sys_call
>>      do_syscall_64
>>      entry_SYSCALL_64_after_hwframe
>>     ------------[ end trace ]------------
>>
>> Signed-off-by: Bojun Zhu <zhubojun.zbj@antgroup.com>
>> ---
>>  arch/x86/kernel/cpu/sgx/ioctl.c | 40 +++++++++++++++++++++++----------
>>  1 file changed, 28 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c  
>> b/arch/x86/kernel/cpu/sgx/ioctl.c
>> index b65ab214bdf5..6199f483143e 100644
>> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
>> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
>> @@ -365,6 +365,20 @@ static int sgx_validate_offset_length(struct  
>> sgx_encl *encl,
>>  	return 0;
>>  }
>> +/*
>> + * Check signals and invoke scheduler. Return true for a pending  
>> signal.
>> + */
>> +static bool sgx_check_signal_and_resched(void)
>> +{
>> +	if (signal_pending(current))
>> +		return true;
>> +
>> +	if (need_resched())
>> +		cond_resched();
>> +
>> +	return false;
>> +}
>> +
>>  /**
>>   * sgx_ioc_enclave_add_pages() - The handler for  
>> %SGX_IOC_ENCLAVE_ADD_PAGES
>>   * @encl:       an enclave pointer
>> @@ -409,7 +423,7 @@ static long sgx_ioc_enclave_add_pages(struct  
>> sgx_encl *encl, void __user *arg)
>>  	struct sgx_enclave_add_pages add_arg;
>>  	struct sgx_secinfo secinfo;
>>  	unsigned long c;
>> -	int ret;
>> +	int ret = -ERESTARTSYS;
>> 	if (!test_bit(SGX_ENCL_CREATED, &encl->flags) ||
>>  	    test_bit(SGX_ENCL_INITIALIZED, &encl->flags))
>> @@ -432,15 +446,8 @@ static long sgx_ioc_enclave_add_pages(struct  
>> sgx_encl *encl, void __user *arg)
>>  		return -EINVAL;
>> 	for (c = 0 ; c < add_arg.length; c += PAGE_SIZE) {
>> -		if (signal_pending(current)) {
>> -			if (!c)
>> -				ret = -ERESTARTSYS;
>> -
>> +		if (sgx_check_signal_and_resched())
>>  			break;
>> -		}
>
> ERESTARTSYS is only appropriate if we have not EADDed any pages yet.
> If we got interrupted in the middle, we should return 0. User space  
> would check the 'count' returned and decide to recall this ioctl() with  
> 'offset'  reset to the next page, and adjust length.

NVM, I misread it. ret will be changed to zero in subsequent iteration.

Reviewed-by: Haitao Huang <haitao.huang@linux.intel.com>

Thanks
Haitao
Jarkko Sakkinen May 16, 2024, 8:24 a.m. UTC | #4
On Thu May 16, 2024 at 12:55 AM EEST, Haitao Huang wrote:
> On Wed, 15 May 2024 01:55:21 -0500, Bojun Zhu <zhubojun.zbj@antgroup.com>  
> wrote:
>
> > EDMM's ioctl()s support batch operations, which may be
> > time-consuming. Try to explicitly give up the CPU as the prefix
> > operation at the every begin of "for loop" in
> > sgx_enclave_{ modify_types | restrict_permissions | remove_pages}
> > to give other tasks a chance to run, and avoid softlockup warning.
> >
> > Additionally perform pending signals check as the prefix operation,
> > and introduce sgx_check_signal_and_resched(),
> > which wraps all the checks.
> >
> > The following has been observed on Linux v6.9-rc5 with kernel
> > preemptions disabled(by configuring "PREEMPT_NONE=y"), when kernel
> > is requested to restrict page permissions of a large number of EPC pages.
> >
> >     ------------[ cut here ]------------
> >     watchdog: BUG: soft lockup - CPU#45 stuck for 22s!
> >     ...
> >     RIP: 0010:sgx_enclave_restrict_permissions+0xba/0x1f0
> >     ...
> >     Call Trace:
> >      sgx_ioctl
> >      __x64_sys_ioctl
> >      x64_sys_call
> >      do_syscall_64
> >      entry_SYSCALL_64_after_hwframe
> >     ------------[ end trace ]------------
> >
> > Signed-off-by: Bojun Zhu <zhubojun.zbj@antgroup.com>
> > ---
> >  arch/x86/kernel/cpu/sgx/ioctl.c | 40 +++++++++++++++++++++++----------
> >  1 file changed, 28 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c  
> > b/arch/x86/kernel/cpu/sgx/ioctl.c
> > index b65ab214bdf5..6199f483143e 100644
> > --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> > +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> > @@ -365,6 +365,20 @@ static int sgx_validate_offset_length(struct  
> > sgx_encl *encl,
> >  	return 0;
> >  }
> > +/*
> > + * Check signals and invoke scheduler. Return true for a pending signal.
> > + */
> > +static bool sgx_check_signal_and_resched(void)
> > +{
> > +	if (signal_pending(current))
> > +		return true;
> > +
> > +	if (need_resched())
> > +		cond_resched();
> > +
> > +	return false;
> > +}
> > +
> >  /**
> >   * sgx_ioc_enclave_add_pages() - The handler for  
> > %SGX_IOC_ENCLAVE_ADD_PAGES
> >   * @encl:       an enclave pointer
> > @@ -409,7 +423,7 @@ static long sgx_ioc_enclave_add_pages(struct  
> > sgx_encl *encl, void __user *arg)
> >  	struct sgx_enclave_add_pages add_arg;
> >  	struct sgx_secinfo secinfo;
> >  	unsigned long c;
> > -	int ret;
> > +	int ret = -ERESTARTSYS;
> > 	if (!test_bit(SGX_ENCL_CREATED, &encl->flags) ||
> >  	    test_bit(SGX_ENCL_INITIALIZED, &encl->flags))
> > @@ -432,15 +446,8 @@ static long sgx_ioc_enclave_add_pages(struct  
> > sgx_encl *encl, void __user *arg)
> >  		return -EINVAL;
> > 	for (c = 0 ; c < add_arg.length; c += PAGE_SIZE) {
> > -		if (signal_pending(current)) {
> > -			if (!c)
> > -				ret = -ERESTARTSYS;
> > -
> > +		if (sgx_check_signal_and_resched())
> >  			break;
> > -		}
>
> ERESTARTSYS is only appropriate if we have not EADDed any pages yet.
> If we got interrupted in the middle, we should return 0. User space would  
> check the 'count' returned and decide to recall this ioctl() with  
> 'offset'  reset to the next page, and adjust length.

Good catch! Thanks Haitao for the remark.

BR, Jarkko
Jarkko Sakkinen May 16, 2024, 8:26 a.m. UTC | #5
On Thu May 16, 2024 at 1:29 AM EEST, Haitao Huang wrote:
> On Wed, 15 May 2024 16:55:59 -0500, Haitao Huang  
> <haitao.huang@linux.intel.com> wrote:
>
> > On Wed, 15 May 2024 01:55:21 -0500, Bojun Zhu  
> > <zhubojun.zbj@antgroup.com> wrote:
> >
> >> EDMM's ioctl()s support batch operations, which may be
> >> time-consuming. Try to explicitly give up the CPU as the prefix
> >> operation at the every begin of "for loop" in
> >> sgx_enclave_{ modify_types | restrict_permissions | remove_pages}
> >> to give other tasks a chance to run, and avoid softlockup warning.
> >>
> >> Additionally perform pending signals check as the prefix operation,
> >> and introduce sgx_check_signal_and_resched(),
> >> which wraps all the checks.
> >>
> >> The following has been observed on Linux v6.9-rc5 with kernel
> >> preemptions disabled(by configuring "PREEMPT_NONE=y"), when kernel
> >> is requested to restrict page permissions of a large number of EPC  
> >> pages.
> >>
> >>     ------------[ cut here ]------------
> >>     watchdog: BUG: soft lockup - CPU#45 stuck for 22s!
> >>     ...
> >>     RIP: 0010:sgx_enclave_restrict_permissions+0xba/0x1f0
> >>     ...
> >>     Call Trace:
> >>      sgx_ioctl
> >>      __x64_sys_ioctl
> >>      x64_sys_call
> >>      do_syscall_64
> >>      entry_SYSCALL_64_after_hwframe
> >>     ------------[ end trace ]------------
> >>
> >> Signed-off-by: Bojun Zhu <zhubojun.zbj@antgroup.com>
> >> ---
> >>  arch/x86/kernel/cpu/sgx/ioctl.c | 40 +++++++++++++++++++++++----------
> >>  1 file changed, 28 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c  
> >> b/arch/x86/kernel/cpu/sgx/ioctl.c
> >> index b65ab214bdf5..6199f483143e 100644
> >> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> >> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> >> @@ -365,6 +365,20 @@ static int sgx_validate_offset_length(struct  
> >> sgx_encl *encl,
> >>  	return 0;
> >>  }
> >> +/*
> >> + * Check signals and invoke scheduler. Return true for a pending  
> >> signal.
> >> + */
> >> +static bool sgx_check_signal_and_resched(void)
> >> +{
> >> +	if (signal_pending(current))
> >> +		return true;
> >> +
> >> +	if (need_resched())
> >> +		cond_resched();
> >> +
> >> +	return false;
> >> +}
> >> +
> >>  /**
> >>   * sgx_ioc_enclave_add_pages() - The handler for  
> >> %SGX_IOC_ENCLAVE_ADD_PAGES
> >>   * @encl:       an enclave pointer
> >> @@ -409,7 +423,7 @@ static long sgx_ioc_enclave_add_pages(struct  
> >> sgx_encl *encl, void __user *arg)
> >>  	struct sgx_enclave_add_pages add_arg;
> >>  	struct sgx_secinfo secinfo;
> >>  	unsigned long c;
> >> -	int ret;
> >> +	int ret = -ERESTARTSYS;
> >> 	if (!test_bit(SGX_ENCL_CREATED, &encl->flags) ||
> >>  	    test_bit(SGX_ENCL_INITIALIZED, &encl->flags))
> >> @@ -432,15 +446,8 @@ static long sgx_ioc_enclave_add_pages(struct  
> >> sgx_encl *encl, void __user *arg)
> >>  		return -EINVAL;
> >> 	for (c = 0 ; c < add_arg.length; c += PAGE_SIZE) {
> >> -		if (signal_pending(current)) {
> >> -			if (!c)
> >> -				ret = -ERESTARTSYS;
> >> -
> >> +		if (sgx_check_signal_and_resched())
> >>  			break;
> >> -		}
> >
> > ERESTARTSYS is only appropriate if we have not EADDed any pages yet.
> > If we got interrupted in the middle, we should return 0. User space  
> > would check the 'count' returned and decide to recall this ioctl() with  
> > 'offset'  reset to the next page, and adjust length.
>
> NVM, I misread it. ret will be changed to zero in subsequent iteration.
>
> Reviewed-by: Haitao Huang <haitao.huang@linux.intel.com>

Duh, and I responded too quickly. OK, I revisited the original
patch and yes ret gets reseted. Ignore my previous response ;-)

My tags still hold, sorry.

BR, Jarkko
diff mbox series

Patch

diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index b65ab214bdf5..6199f483143e 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -365,6 +365,20 @@  static int sgx_validate_offset_length(struct sgx_encl *encl,
 	return 0;
 }
 
+/*
+ * Check signals and invoke scheduler. Return true for a pending signal.
+ */
+static bool sgx_check_signal_and_resched(void)
+{
+	if (signal_pending(current))
+		return true;
+
+	if (need_resched())
+		cond_resched();
+
+	return false;
+}
+
 /**
  * sgx_ioc_enclave_add_pages() - The handler for %SGX_IOC_ENCLAVE_ADD_PAGES
  * @encl:       an enclave pointer
@@ -409,7 +423,7 @@  static long sgx_ioc_enclave_add_pages(struct sgx_encl *encl, void __user *arg)
 	struct sgx_enclave_add_pages add_arg;
 	struct sgx_secinfo secinfo;
 	unsigned long c;
-	int ret;
+	int ret = -ERESTARTSYS;
 
 	if (!test_bit(SGX_ENCL_CREATED, &encl->flags) ||
 	    test_bit(SGX_ENCL_INITIALIZED, &encl->flags))
@@ -432,15 +446,8 @@  static long sgx_ioc_enclave_add_pages(struct sgx_encl *encl, void __user *arg)
 		return -EINVAL;
 
 	for (c = 0 ; c < add_arg.length; c += PAGE_SIZE) {
-		if (signal_pending(current)) {
-			if (!c)
-				ret = -ERESTARTSYS;
-
+		if (sgx_check_signal_and_resched())
 			break;
-		}
-
-		if (need_resched())
-			cond_resched();
 
 		ret = sgx_encl_add_page(encl, add_arg.src + c, add_arg.offset + c,
 					&secinfo, add_arg.flags);
@@ -740,12 +747,15 @@  sgx_enclave_restrict_permissions(struct sgx_encl *encl,
 	unsigned long addr;
 	unsigned long c;
 	void *epc_virt;
-	int ret;
+	int ret = -ERESTARTSYS;
 
 	memset(&secinfo, 0, sizeof(secinfo));
 	secinfo.flags = modp->permissions & SGX_SECINFO_PERMISSION_MASK;
 
 	for (c = 0 ; c < modp->length; c += PAGE_SIZE) {
+		if (sgx_check_signal_and_resched())
+			goto out;
+
 		addr = encl->base + modp->offset + c;
 
 		sgx_reclaim_direct();
@@ -898,7 +908,7 @@  static long sgx_enclave_modify_types(struct sgx_encl *encl,
 	unsigned long addr;
 	unsigned long c;
 	void *epc_virt;
-	int ret;
+	int ret = -ERESTARTSYS;
 
 	page_type = modt->page_type & SGX_PAGE_TYPE_MASK;
 
@@ -913,6 +923,9 @@  static long sgx_enclave_modify_types(struct sgx_encl *encl,
 	secinfo.flags = page_type << 8;
 
 	for (c = 0 ; c < modt->length; c += PAGE_SIZE) {
+		if (sgx_check_signal_and_resched())
+			goto out;
+
 		addr = encl->base + modt->offset + c;
 
 		sgx_reclaim_direct();
@@ -1095,12 +1108,15 @@  static long sgx_encl_remove_pages(struct sgx_encl *encl,
 	unsigned long addr;
 	unsigned long c;
 	void *epc_virt;
-	int ret;
+	int ret = -ERESTARTSYS;
 
 	memset(&secinfo, 0, sizeof(secinfo));
 	secinfo.flags = SGX_SECINFO_R | SGX_SECINFO_W | SGX_SECINFO_X;
 
 	for (c = 0 ; c < params->length; c += PAGE_SIZE) {
+		if (sgx_check_signal_and_resched())
+			goto out;
+
 		addr = encl->base + params->offset + c;
 
 		sgx_reclaim_direct();