From: ZhangGuoDong zhangguodong@kylinos.cn
[ Upstream commit f416c556997aa56ec4384c6b6efd6a0e6ac70aa7 ]
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 Acked-by: Namjae Jeon linkinjeon@kernel.org Signed-off-by: Steve French stfrench@microsoft.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
## Commit Analysis: smb/server: fix refcount leak in smb2_open()
### 1. COMMIT MESSAGE ANALYSIS
The commit message is clear and concise: - **Subject:** Explicitly identifies this as a "fix" for a "refcount leak" - **Body:** Explains the bug - when `ksmbd_vfs_getattr()` fails, the reference count of `ksmbd_file` is not being released - **Tags:** Has Suggested-by from Namjae Jeon (ksmbd maintainer), Acked- by from same maintainer, and signed off by Steve French (SMB/CIFS maintainer)
The maintainer involvement indicates this is a legitimate bug fix.
### 2. CODE CHANGE ANALYSIS
Let me examine the exact change:
```c // BEFORE: rc = ksmbd_vfs_getattr(&fp->filp->f_path, &stat); if (rc) goto err_out2;
ksmbd_put_durable_fd(fp); goto reconnected_fp;
// AFTER: rc = ksmbd_vfs_getattr(&fp->filp->f_path, &stat); ksmbd_put_durable_fd(fp); if (rc) goto err_out2;
goto reconnected_fp; ```
**The Bug:** In the original code, when `ksmbd_vfs_getattr()` fails (returns non- zero), the code jumps directly to `err_out2` **without** calling `ksmbd_put_durable_fd(fp)`. This means the reference count acquired earlier (via `ksmbd_reopen_durable_fd`) is never released - a classic refcount leak.
**The Fix:** Move `ksmbd_put_durable_fd(fp)` to before the error check so the reference is released in both success and error paths. This is the correct pattern for resource cleanup.
### 3. CLASSIFICATION
- **Type:** Bug fix (resource leak) - **Severity:** Medium - refcount leaks can lead to memory exhaustion over time - **Component:** ksmbd (kernel SMB server) - fs/smb/server/
### 4. SCOPE AND RISK ASSESSMENT
- **Lines changed:** 2 (one line moved) - **Files changed:** 1 (fs/smb/server/smb2pdu.c) - **Complexity:** Very low - straightforward line repositioning - **Risk:** Very low - the fix is obviously correct and doesn't change any logic, just ensures cleanup happens unconditionally
### 5. USER IMPACT
- **Affected users:** Systems running ksmbd (kernel SMB server) - **Trigger condition:** When a durable file handle reconnection attempt has `ksmbd_vfs_getattr()` fail - **Consequence of bug:** Reference count leak leading to potential resource exhaustion over time - **Severity:** Important for production SMB servers that may run for extended periods
### 6. STABILITY INDICATORS
- ✅ Acked-by from subsystem maintainer (Namjae Jeon) - ✅ Suggested-by from the same maintainer - ✅ Signed-off by Steve French (CIFS/SMB maintainer) - ✅ Small, surgical fix with clear scope
### 7. DEPENDENCY CHECK
This is a standalone fix. The code path exists in all kernels with ksmbd support (5.15+). No other commits are required for this fix to work.
### STABLE KERNEL CRITERIA ASSESSMENT
| Criterion | Status | |-----------|--------| | Obviously correct | ✅ Yes - reference must be released in both paths | | Fixes real bug | ✅ Yes - refcount leak is a real resource management bug | | Important issue | ✅ Yes - can cause resource exhaustion | | Small and contained | ✅ Yes - 2 lines, single file | | No new features | ✅ Yes - pure bugfix | | Applies cleanly | ✅ Should apply cleanly |
### CONCLUSION
This is an ideal stable backport candidate: - **Clear bug:** Reference count leak when error path is taken - **Obvious fix:** Move cleanup before error check - **Minimal change:** Single line repositioned - **Low risk:** No logic changes, just proper resource management - **Maintainer endorsed:** Acked by ksmbd maintainer
The fix addresses a resource leak that could affect production SMB servers over time. The change is trivial (one line moved), obviously correct, and has zero risk of introducing regressions.
**YES**
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 e052dcb9a14c..3795fb90e161 100644 --- a/fs/smb/server/smb2pdu.c +++ b/fs/smb/server/smb2pdu.c @@ -3019,10 +3019,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)