[BUG] x86/sgx: missing kref_put() in sgx_encl_mm_add() error path

[BUG] x86/sgx: missing kref_put() in sgx_encl_mm_add() error path

[Dingisoul]

Hi Kernel maintainers,

We found a possible refcount leak in sgx_encl_mm_add().

In this function, a reference to encl->refcount is taken before registering the MMU notifier:

/* Grab a refcount for the encl_mm->encl reference: */
kref_get(&encl->refcount);  // 1. Reference acquired here.
encl_mm->encl = encl;

ret = __mmu_notifier_register(&encl_mm->mmu_notifier, mm);
if (ret) {
	kfree(encl_mm);     
	return ret;         // 2. Returns without kref_put.
}

If __mmu_notifier_register() fails, the function frees encl_mm but does not drop the reference acquired by kref_get(&encl->refcount). This seems to leak one reference to encl.

Please let us know if the kref_put is unnecessary here.

Thanks.

Re: [BUG] x86/sgx: missing kref_put() in sgx_encl_mm_add() error path

[Dave Hansen]

On 3/30/26 14:09, Dingisoul wrote:
> If __mmu_notifier_register() fails, the function frees encl_mm but
> does not drop the reference acquired by kref_get(&encl->refcount).
> This seems to leak one reference to encl.

Yep it looks like a leak.

> Please let us know if the kref_put is unnecessary here.

The easiest way to fix it is to just move the kref_get() until after the
notifier has been registered. There's no risk of encl going away in this
context, so it doesn't matter when the kref_get() happens as long as it
_does_ happen.

Re: Re: [BUG] x86/sgx: missing kref_put() in sgx_encl_mm_add() error path

[Dingisoul]

Hi Dave,

Thanks for the confirmation and the suggestion. 

Moving the kref_get() after the successful mmu_notifier 
registration looks great and keeps the error path clean. 

Should we send a formal patch?

Re: [BUG] x86/sgx: missing kref_put() in sgx_encl_mm_add() error path

[Jarkko Sakkinen]

On Mon, Mar 30, 2026 at 05:09:57PM -0400, Dingisoul wrote:
> Hi Kernel maintainers,
> 
> We found a possible refcount leak in sgx_encl_mm_add().
> 
> In this function, a reference to encl->refcount is taken before registering the MMU notifier:
> 
> /* Grab a refcount for the encl_mm->encl reference: */
> kref_get(&encl->refcount);  // 1. Reference acquired here.
> encl_mm->encl = encl;
> 
> ret = __mmu_notifier_register(&encl_mm->mmu_notifier, mm);
> if (ret) {
> 	kfree(encl_mm);     
> 	return ret;         // 2. Returns without kref_put.
> }
> 
> If __mmu_notifier_register() fails, the function frees encl_mm but does not drop the reference acquired by kref_get(&encl->refcount). This seems to leak one reference to encl.
> 
> Please let us know if the kref_put is unnecessary here.
> 
> Thanks.

It is a leak as encl's, stored in file, will retain increased refcount.

BR, Jarkko