Bug 295577 - smbfs: smbfs_node_alloc() leaks name buffers when insmntque() fails
Summary: smbfs: smbfs_node_alloc() leaks name buffers when insmntque() fails
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 14.4-RELEASE
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-fs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2026-05-25 14:34 UTC by Haoxiang Li
Modified: 2026-06-04 00:42 UTC (History)
1 user (show)

See Also:


Attachments
Fix smbfs_node_alloc() cleanup on insmntque() failure (613 bytes, patch)
2026-05-25 14:34 UTC, Haoxiang Li
no flags Details | Diff
Commit candidate (975 bytes, patch)
2026-05-26 13:57 UTC, Konstantin Belousov
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Haoxiang Li 2026-05-25 14:34:13 UTC
Created attachment 271120 [details]
Fix smbfs_node_alloc() cleanup on insmntque() failure

smbfs_node_alloc() leaks np->n_rpath and np->n_name on the insmntque()
failure path. I tested this on FreeBSD 14.4-RELEASE, but the affected code
path does not appear to be specific to this version.

bug #265148, comment #2 discussed a related situation. That reasoning is correct for the vfs_hash_insert() failure path. However, before insmntque() calls vgone(), vp->v_op has already been changed to dead_vnodeops. Therefore, VOP_RECLAIM resolves to VOP_NULL rather than smbfs_reclaim(), and the resources owned by np are not reclaimed on this path.

I tested this in a FreeBSD 14.4-RELEASE amd64 QEMU VM using fault injection that temporarily sets MNTK_UNMOUNT | MNTK_UNMOUNTF before calling insmntque(), forcing the real insmntque1_int() EBUSY path.

After the failed mount, vmstat -m showed:

      Type           Use  Memory  Req  Size(s)
      smbufs_node      0       0    1  256
      smbufs_nname     2      32    2  16

This shows that np was freed, but the two M_SMBNODENAME allocations, np->n_rpath and np->n_name, remained active.

I also tested the vfs_hash_insert() duplicate-vnode path separately. That path does not appear to leak, because vfs_hash_insert() calls vgone(vp) while vp->v_op is still smbfs_vnodeops, so smbfs_reclaim() runs and frees the smbnode resources. Repeated forced vfs_hash_insert() duplicate insertions increased the Req counters but did not increase the active Use counters. After unmount, both smbufs_node and smbufs_nname Use returned to zero.

Thus, cleanup is necessary when insmntque() fails.
Comment 1 Konstantin Belousov freebsd_committer freebsd_triage 2026-05-26 13:57:42 UTC
Created attachment 271190 [details]
Commit candidate

This is what I intent to commit.
The checks for NULL pointers passed to free() are removed.
The sm_didrele assignment is not needed, it only matters for reclaims done during
unmount.

Recheck the author name in the commit.
Comment 2 Haoxiang Li 2026-05-27 01:51:00 UTC
(In reply to Konstantin Belousov from comment #1)
Thank you very much for your modifications!

The correct author name should be Haoxiang Li. I have updated my name in the Bugzilla settings, but I am not sure how to update the author information in the commit. I would appreciate your help with this.

Thanks again!
Comment 3 Haoxiang Li 2026-05-27 01:54:56 UTC
Comment on attachment 271190 [details]
Commit candidate

>From 7781f0278a29a31753e2f9795af40c598362e974 Mon Sep 17 00:00:00 2001
>From: Haoxiang Li <lihaoxiang@isrc.iscas.ac.cn>
>Date: Tue, 26 May 2026 16:21:39 +0300
>Subject: [PATCH] smbfs: plug smbfs_node_alloc() leak of name buffers when
> insmntque() fails
>
>PR:	295577
>MFC after:	2 weeks
>---
> sys/fs/smbfs/smbfs_node.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/sys/fs/smbfs/smbfs_node.c b/sys/fs/smbfs/smbfs_node.c
>index 9893987f7ccd..b2b5d63e2eaf 100644
>--- a/sys/fs/smbfs/smbfs_node.c
>+++ b/sys/fs/smbfs/smbfs_node.c
>@@ -212,6 +212,10 @@ smbfs_node_alloc(struct mount *mp, struct vnode *dvp, const char *dirnm,
> 		SMBERROR("new vnode '%s' born without parent ?\n", np->n_name);
> 	error = insmntque(vp, mp);
> 	if (error) {
>+		smbfs_name_free(np->n_name);
>+		free(np->n_rpath, M_SMBNODENAME);
>+		if (np->n_parent != NULL && (np->n_flag & NREFPARENT) != 0)
>+			vrele(np->n_parent);
> 		free(np, M_SMBNODE);
> 		return (error);
> 	}
>-- 
>2.54.0
>
Comment 4 commit-hook freebsd_committer freebsd_triage 2026-05-27 08:10:59 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=f0e702a7a289f0f350bdaeb943805d6d17f9e403

commit f0e702a7a289f0f350bdaeb943805d6d17f9e403
Author:     Haoxiang Li <lihaoxiang@isrc.iscas.ac.cn>
AuthorDate: 2026-05-26 13:21:39 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2026-05-27 08:09:31 +0000

    smbfs: plug smbfs_node_alloc() leak of name buffers when insmntque() fails

    PR:     295577
    MFC after:      2 weeks

 sys/fs/smbfs/smbfs_node.c | 4 ++++
 1 file changed, 4 insertions(+)
Comment 5 commit-hook freebsd_committer freebsd_triage 2026-06-04 00:36:45 UTC
A commit in branch stable/15 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=21fb5a5e61c333131cfbb4b522db00142a905707

commit 21fb5a5e61c333131cfbb4b522db00142a905707
Author:     Haoxiang Li <lihaoxiang@isrc.iscas.ac.cn>
AuthorDate: 2026-05-26 13:21:39 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2026-06-04 00:24:13 +0000

    smbfs: plug smbfs_node_alloc() leak of name buffers when insmntque() fails

    PR:     295577

    (cherry picked from commit f0e702a7a289f0f350bdaeb943805d6d17f9e403)

 sys/fs/smbfs/smbfs_node.c | 4 ++++
 1 file changed, 4 insertions(+)