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

[ChenXiaoSong <chenxiaosong@chenxiaosong.com>]

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.
>>