[PATCH v2 0/1] smb/server: fix some refcount leaks

[PATCH v2 0/1] smb/server: fix some refcount leaks

[chenxiaosong.chenxiaosong]

From: ChenXiaoSong <chenxiaosong@kylinos.cn>

v1: https://lore.kernel.org/linux-cifs/20251229021330.1026506-1-chenxiaosong.chenxiaosong@linux.dev/
v1->v2:
  - Call ksmbd_put_durable_fd() immediately after ksmbd_vfs_getattr() has completed.

The following patches from v1 have already been merged into ksmbd.git ksmbd-for-next-next:
 - 0d762babd1db smb/server: fix refcount leak in parse_durable_handle_context()

My dear team member, ZhangGuoDong, has caught some refcount leak issues.

Other patches pending review (including those from others) can be found at the following link:
https://chenxiaosong.com/en/smb-patch.html

ZhangGuoDong (1):
  smb/server: fix refcount leak in smb2_open()

 fs/smb/server/smb2pdu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.43.0

[PATCH v2 1/1] smb/server: fix refcount leak in smb2_open()

[chenxiaosong.chenxiaosong]

From: ZhangGuoDong <zhangguodong@kylinos.cn>

When ksmbd_vfs_getattr() fails, the reference count of ksmbd_file
must be released.

Suggested-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: ZhangGuoDong <zhangguodong@kylinos.cn>
Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
---
 fs/smb/server/smb2pdu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 6a966c696f7d..dc730fe348e4 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -3007,10 +3007,10 @@ int smb2_open(struct ksmbd_work *work)
 			file_info = FILE_OPENED;
 
 			rc = ksmbd_vfs_getattr(&fp->filp->f_path, &stat);
+			ksmbd_put_durable_fd(fp);
 			if (rc)
 				goto err_out2;
 
-			ksmbd_put_durable_fd(fp);
 			goto reconnected_fp;
 		}
 	} else if (req_op_level == SMB2_OPLOCK_LEVEL_LEASE)
-- 
2.43.0

Re: [PATCH v2 1/1] smb/server: fix refcount leak in smb2_open()

[Namjae Jeon]

On Mon, Dec 29, 2025 at 12:16 PM <chenxiaosong.chenxiaosong@linux.dev> wrote:
>
> From: ZhangGuoDong <zhangguodong@kylinos.cn>
>
> When ksmbd_vfs_getattr() fails, the reference count of ksmbd_file
> must be released.
>
> Suggested-by: Namjae Jeon <linkinjeon@kernel.org>
> Signed-off-by: ZhangGuoDong <zhangguodong@kylinos.cn>
> Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Applied it to #ksmbd-for-next-next.
Thanks!

Re: [PATCH v2 1/1] smb/server: fix refcount leak in smb2_open()

[Guenter Roeck]

Hi,

On Mon, Dec 29, 2025 at 11:15:18AM +0800, chenxiaosong.chenxiaosong@linux.dev wrote:
> From: ZhangGuoDong <zhangguodong@kylinos.cn>
> 
> When ksmbd_vfs_getattr() fails, the reference count of ksmbd_file
> must be released.
> 
> Suggested-by: Namjae Jeon <linkinjeon@kernel.org>
> Signed-off-by: ZhangGuoDong <zhangguodong@kylinos.cn>
> Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
> ---
>  fs/smb/server/smb2pdu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
> index 6a966c696f7d..dc730fe348e4 100644
> --- a/fs/smb/server/smb2pdu.c
> +++ b/fs/smb/server/smb2pdu.c
> @@ -3007,10 +3007,10 @@ int smb2_open(struct ksmbd_work *work)
>  			file_info = FILE_OPENED;
>  
>  			rc = ksmbd_vfs_getattr(&fp->filp->f_path, &stat);
> +			ksmbd_put_durable_fd(fp);
>  			if (rc)
>  				goto err_out2;
>  
> -			ksmbd_put_durable_fd(fp);
>  			goto reconnected_fp;
>  		}
>  	} else if (req_op_level == SMB2_OPLOCK_LEVEL_LEASE)
> -- 

Running an experimental AI agent on this patch produced the following
feedback:

This isn't a bug introduced by your patch, but it looks like there is still a
resource leak here. If ksmbd_override_fsids() fails, we jump to err_out2.
At that point, fp is NULL because it hasn't been assigned dh_info.fp yet,
so ksmbd_fd_put(work, fp) will not be called. However, dh_info.fp was
already inserted into the session file table by ksmbd_reopen_durable_fd(),
so it will leak in the session file table until the session is closed.
Should fp = dh_info.fp; be moved before the ksmbd_override_fsids() check?

PTAL and let me know if it has a point or if it is missing something.

Thanks!

Guenter

Re: [PATCH v2 1/1] smb/server: fix refcount leak in smb2_open()

[ChenXiaoSong]

Hi Guenter,

Thank you for taking the time to look into this issue.

I reviewed the relevant code in more detail and did not find any leak.

Both `ksmbd_put_durable_fd()` and `ksmbd_fd_put()` will eventually call 
`__ksmbd_remove_fd()` (remove fd from file table).

If my understanding is incorrect, please let me know.

Thanks,
ChenXiaoSong <chenxiaosong@chenxiaosong.com>

在 2026/2/26 00:49, Guenter Roeck 写道:
> Running an experimental AI agent on this patch produced the following
> feedback:
> 
> This isn't a bug introduced by your patch, but it looks like there is still a
> resource leak here. If ksmbd_override_fsids() fails, we jump to err_out2.
> At that point, fp is NULL because it hasn't been assigned dh_info.fp yet,
> so ksmbd_fd_put(work, fp) will not be called. However, dh_info.fp was
> already inserted into the session file table by ksmbd_reopen_durable_fd(),
> so it will leak in the session file table until the session is closed.
> Should fp = dh_info.fp; be moved before the ksmbd_override_fsids() check?
> 
> PTAL and let me know if it has a point or if it is missing something.

Re: [PATCH v2 1/1] smb/server: fix refcount leak in smb2_open()

[Guenter Roeck]

Hi,

On 2/25/26 20:12, ChenXiaoSong wrote:
> Hi Guenter,
> 
> Thank you for taking the time to look into this issue.
> 
> I reviewed the relevant code in more detail and did not find any leak.
> 
> Both `ksmbd_put_durable_fd()` and `ksmbd_fd_put()` will eventually call `__ksmbd_remove_fd()` (remove fd from file table).
> 
Thanks a lot for taking the time and checking the code.

> If my understanding is incorrect, please let me know.
> 

Just to make sure, I'll have another look at the code and provide
your feedback to the AI agent.

Again, thanks a lot for your time.

Guenter

> Thanks,
> ChenXiaoSong <chenxiaosong@chenxiaosong.com>
> 
> 在 2026/2/26 00:49, Guenter Roeck 写道:
>> Running an experimental AI agent on this patch produced the following
>> feedback:
>>
>> This isn't a bug introduced by your patch, but it looks like there is still a
>> resource leak here. If ksmbd_override_fsids() fails, we jump to err_out2.
>> At that point, fp is NULL because it hasn't been assigned dh_info.fp yet,
>> so ksmbd_fd_put(work, fp) will not be called. However, dh_info.fp was
>> already inserted into the session file table by ksmbd_reopen_durable_fd(),
>> so it will leak in the session file table until the session is closed.
>> Should fp = dh_info.fp; be moved before the ksmbd_override_fsids() check?
>>
>> PTAL and let me know if it has a point or if it is missing something.
> 

Re: [PATCH v2 1/1] smb/server: fix refcount leak in smb2_open()

[Guenter Roeck]

Hi,

On 2/25/26 20:12, ChenXiaoSong wrote:
> Hi Guenter,
> 
> Thank you for taking the time to look into this issue.
> 
> I reviewed the relevant code in more detail and did not find any leak.
> 
> Both `ksmbd_put_durable_fd()` and `ksmbd_fd_put()` will eventually call `__ksmbd_remove_fd()` (remove fd from file table).
> 

Sorry for bothering you again. Are you sure ?

ksmbd_put_durable_fd() calls __ksmbd_close_fd() with NULL first parameter.
__ksmbd_close_fd() only calls __ksmbd_remove_fd() if ft (the first parameter)
is not NULL.

ksmbd_fd_put() does call __ksmbd_remove_fd() with first parameter, but ...

> If my understanding is incorrect, please let me know.
> 
> Thanks,
> ChenXiaoSong <chenxiaosong@chenxiaosong.com>
> 
> 在 2026/2/26 00:49, Guenter Roeck 写道:
>> Running an experimental AI agent on this patch produced the following
>> feedback:
>>
>> This isn't a bug introduced by your patch, but it looks like there is still a
>> resource leak here. If ksmbd_override_fsids() fails, we jump to err_out2.
>> At that point, fp is NULL because it hasn't been assigned dh_info.fp yet,
>> so ksmbd_fd_put(work, fp) will not be called. However, dh_info.fp was

the AI is trying to make the point that ksmbd_fd_put() will not be called
because fp == NULL.

Thanks,
Guenter

>> already inserted into the session file table by ksmbd_reopen_durable_fd(),
>> so it will leak in the session file table until the session is closed.
>> Should fp = dh_info.fp; be moved before the ksmbd_override_fsids() check?
>>
>> PTAL and let me know if it has a point or if it is missing something.
> 

Re: [PATCH v2 1/1] smb/server: fix refcount leak in smb2_open()

[ChenXiaoSong]

Hi Guenter,

Sorry for the late reply. I had some family matters to take care of 
yesterday.

Please see the following process:

```
smb2_open
   smb2_check_durable_oplock
     opinfo_get(fp) // inc refcount
   ksmbd_reopen_durable_fd
     __open_id(&work->sess->file_table, fp,
       idr_alloc_cyclic(ft->idr, fp, ...)
       __open_id_set(fp, id, type); // insert into file table
   ksmbd_override_fsids // fail
   ksmbd_put_durable_fd
     __ksmbd_close_fd
       __ksmbd_remove_fd  // remove dh_info.fp from file table
   // dh_info.fp has already been removed from the file table
   ksmbd_fd_put(..., fp) // fp == NULL
```

Thanks,
ChenXiaoSong <chenxiaosong@chenxiaosong.com>

On 2026/2/26 13:09, Guenter Roeck wrote:
> Hi,
> 
> On 2/25/26 20:12, ChenXiaoSong wrote:
>> Hi Guenter,
>>
>> Thank you for taking the time to look into this issue.
>>
>> I reviewed the relevant code in more detail and did not find any leak.
>>
>> Both `ksmbd_put_durable_fd()` and `ksmbd_fd_put()` will eventually 
>> call `__ksmbd_remove_fd()` (remove fd from file table).
>>
> 
> Sorry for bothering you again. Are you sure ?
> 
> ksmbd_put_durable_fd() calls __ksmbd_close_fd() with NULL first parameter.
> __ksmbd_close_fd() only calls __ksmbd_remove_fd() if ft (the first 
> parameter)
> is not NULL.
> 
> ksmbd_fd_put() does call __ksmbd_remove_fd() with first parameter, but ...
> 
>> If my understanding is incorrect, please let me know.
>>
>> Thanks,
>> ChenXiaoSong <chenxiaosong@chenxiaosong.com>
>>
>> 在 2026/2/26 00:49, Guenter Roeck 写道:
>>> Running an experimental AI agent on this patch produced the following
>>> feedback:
>>>
>>> This isn't a bug introduced by your patch, but it looks like there is 
>>> still a
>>> resource leak here. If ksmbd_override_fsids() fails, we jump to 
>>> err_out2.
>>> At that point, fp is NULL because it hasn't been assigned dh_info.fp 
>>> yet,
>>> so ksmbd_fd_put(work, fp) will not be called. However, dh_info.fp was
> 
> the AI is trying to make the point that ksmbd_fd_put() will not be called
> because fp == NULL.
> 
> Thanks,
> Guenter
> 
>>> already inserted into the session file table by 
>>> ksmbd_reopen_durable_fd(),
>>> so it will leak in the session file table until the session is closed.
>>> Should fp = dh_info.fp; be moved before the ksmbd_override_fsids() 
>>> check?
>>>
>>> PTAL and let me know if it has a point or if it is missing something.
>>

Re: [PATCH v2 1/1] smb/server: fix refcount leak in smb2_open()

[ChenXiaoSong]

Hi Namjae,

By the way, smb2_open() is over 900 lines long, and we have already 
encountered several memory leaks in it, and there may be more. Perhaps 
we should consider refactoring it to make it easier to maintain.

What do you think?

Thanks,
ChenXiaoSong <chenxiaosong@chenxiaosong.com>

On 2026/2/27 12:07, ChenXiaoSong wrote:
> Hi Guenter,
> 
> Sorry for the late reply. I had some family matters to take care of 
> yesterday.
> 
> Please see the following process:
> 
> ```
> smb2_open
>    smb2_check_durable_oplock
>      opinfo_get(fp) // inc refcount
>    ksmbd_reopen_durable_fd
>      __open_id(&work->sess->file_table, fp,
>        idr_alloc_cyclic(ft->idr, fp, ...)
>        __open_id_set(fp, id, type); // insert into file table
>    ksmbd_override_fsids // fail
>    ksmbd_put_durable_fd
>      __ksmbd_close_fd
>        __ksmbd_remove_fd  // remove dh_info.fp from file table
>    // dh_info.fp has already been removed from the file table
>    ksmbd_fd_put(..., fp) // fp == NULL
> ```

Re: [PATCH v2 1/1] smb/server: fix refcount leak in smb2_open()

[Guenter Roeck]

On 2/26/26 20:07, ChenXiaoSong wrote:
> Hi Guenter,
> 
> Sorry for the late reply. I had some family matters to take care of yesterday.
> 
> Please see the following process:
> 
> ```
> smb2_open
>    smb2_check_durable_oplock
>      opinfo_get(fp) // inc refcount
>    ksmbd_reopen_durable_fd
>      __open_id(&work->sess->file_table, fp,
>        idr_alloc_cyclic(ft->idr, fp, ...)
>        __open_id_set(fp, id, type); // insert into file table
>    ksmbd_override_fsids // fail
>    ksmbd_put_durable_fd
>      __ksmbd_close_fd
>        __ksmbd_remove_fd  // remove dh_info.fp from file table

Yes, but:

void ksmbd_put_durable_fd(struct ksmbd_file *fp)
{
         if (!atomic_dec_and_test(&fp->refcount))
                 return;

         __ksmbd_close_fd(NULL, fp);	// first parameter (ft) is NULL
}

and:

static void __ksmbd_close_fd(struct ksmbd_file_table *ft, struct ksmbd_file *fp)
{
...
	if (ft)	// again, ft is NULL, so __ksmbd_remove_fd() is not called.
                 __ksmbd_remove_fd(ft, fp);

>    // dh_info.fp has already been removed from the file table

I don't think so, but maybe I am missing something. I'll let this go.

Thanks, and sorry for the noise.

Guenter

>    ksmbd_fd_put(..., fp) // fp == NULL
> ```
> 
> Thanks,
> ChenXiaoSong <chenxiaosong@chenxiaosong.com>
> 
> On 2026/2/26 13:09, Guenter Roeck wrote:
>> Hi,
>>
>> On 2/25/26 20:12, ChenXiaoSong wrote:
>>> Hi Guenter,
>>>
>>> Thank you for taking the time to look into this issue.
>>>
>>> I reviewed the relevant code in more detail and did not find any leak.
>>>
>>> Both `ksmbd_put_durable_fd()` and `ksmbd_fd_put()` will eventually call `__ksmbd_remove_fd()` (remove fd from file table).
>>>
>>
>> Sorry for bothering you again. Are you sure ?
>>
>> ksmbd_put_durable_fd() calls __ksmbd_close_fd() with NULL first parameter.
>> __ksmbd_close_fd() only calls __ksmbd_remove_fd() if ft (the first parameter)
>> is not NULL.
>>
>> ksmbd_fd_put() does call __ksmbd_remove_fd() with first parameter, but ...
>>
>>> If my understanding is incorrect, please let me know.
>>>
>>> Thanks,
>>> ChenXiaoSong <chenxiaosong@chenxiaosong.com>
>>>
>>> 在 2026/2/26 00:49, Guenter Roeck 写道:
>>>> Running an experimental AI agent on this patch produced the following
>>>> feedback:
>>>>
>>>> This isn't a bug introduced by your patch, but it looks like there is still a
>>>> resource leak here. If ksmbd_override_fsids() fails, we jump to err_out2.
>>>> At that point, fp is NULL because it hasn't been assigned dh_info.fp yet,
>>>> so ksmbd_fd_put(work, fp) will not be called. However, dh_info.fp was
>>
>> the AI is trying to make the point that ksmbd_fd_put() will not be called
>> because fp == NULL.
>>
>> Thanks,
>> Guenter
>>
>>>> already inserted into the session file table by ksmbd_reopen_durable_fd(),
>>>> so it will leak in the session file table until the session is closed.
>>>> Should fp = dh_info.fp; be moved before the ksmbd_override_fsids() check?
>>>>
>>>> PTAL and let me know if it has a point or if it is missing something.
>>>

Re: [PATCH v2 1/1] smb/server: fix refcount leak in smb2_open()

[Namjae Jeon]

On Fri, Feb 27, 2026 at 1:13 PM ChenXiaoSong
<chenxiaosong@chenxiaosong.com> wrote:
>
> Hi Namjae,
>
> By the way, smb2_open() is over 900 lines long, and we have already
> encountered several memory leaks in it, and there may be more. Perhaps
> we should consider refactoring it to make it easier to maintain.
>
> What do you think?
Agreed, I think we need to factor it out with a helper function...
Thanks.
>
> Thanks,
> ChenXiaoSong <chenxiaosong@chenxiaosong.com>
>
> On 2026/2/27 12:07, ChenXiaoSong wrote:
> > Hi Guenter,
> >
> > Sorry for the late reply. I had some family matters to take care of
> > yesterday.
> >
> > Please see the following process:
> >
> > ```
> > smb2_open
> >    smb2_check_durable_oplock
> >      opinfo_get(fp) // inc refcount
> >    ksmbd_reopen_durable_fd
> >      __open_id(&work->sess->file_table, fp,
> >        idr_alloc_cyclic(ft->idr, fp, ...)
> >        __open_id_set(fp, id, type); // insert into file table
> >    ksmbd_override_fsids // fail
> >    ksmbd_put_durable_fd
> >      __ksmbd_close_fd
> >        __ksmbd_remove_fd  // remove dh_info.fp from file table
> >    // dh_info.fp has already been removed from the file table
> >    ksmbd_fd_put(..., fp) // fp == NULL
> > ```

Re: [PATCH v2 1/1] smb/server: fix refcount leak in smb2_open()

[ChenXiaoSong]

Yes, you are correct. Thank you for pointing this out.

Could you submit a patch to fix this?

Thanks,
ChenXiaoSong <chenxiaosong@chenxiaosong.com>

On 2026/2/27 12:42, Guenter Roeck wrote:
> 
> void ksmbd_put_durable_fd(struct ksmbd_file *fp)
> {
>          if (!atomic_dec_and_test(&fp->refcount))
>                  return;
> 
>          __ksmbd_close_fd(NULL, fp);    // first parameter (ft) is NULL
> }