Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/2228102/?format=api
{ "id": 2228102, "url": "http://patchwork.ozlabs.org/api/patches/2228102/?format=api", "web_url": "http://patchwork.ozlabs.org/project/linux-cifs-client/patch/20260425093829.4004785-3-charsyam@gmail.com/", "project": { "id": 12, "url": "http://patchwork.ozlabs.org/api/projects/12/?format=api", "name": "Linux CIFS Client", "link_name": "linux-cifs-client", "list_id": "linux-cifs.vger.kernel.org", "list_email": "linux-cifs@vger.kernel.org", "web_url": "", "scm_url": "", "webscm_url": "", "list_archive_url": "", "list_archive_url_format": "", "commit_url_format": "" }, "msgid": "<20260425093829.4004785-3-charsyam@gmail.com>", "list_archive_url": null, "date": "2026-04-25T09:38:29", "name": "[2/2] ksmbd: centralize ksmbd_conn final release to plug transport leak", "commit_ref": null, "pull_url": null, "state": "new", "archived": false, "hash": "a29d8374098cf2f0697f8a5942a7408cd2e7ce8a", "submitter": { "id": 93166, "url": "http://patchwork.ozlabs.org/api/people/93166/?format=api", "name": "DaeMyung Kang", "email": "charsyam@gmail.com" }, "delegate": null, "mbox": "http://patchwork.ozlabs.org/project/linux-cifs-client/patch/20260425093829.4004785-3-charsyam@gmail.com/mbox/", "series": [ { "id": 501429, "url": "http://patchwork.ozlabs.org/api/series/501429/?format=api", "web_url": "http://patchwork.ozlabs.org/project/linux-cifs-client/list/?series=501429", "date": "2026-04-25T09:38:27", "name": "ksmbd: fix stop_sessions() iteration and centralize ksmbd_conn release", "version": 1, "mbox": "http://patchwork.ozlabs.org/series/501429/mbox/" } ], "comments": "http://patchwork.ozlabs.org/api/patches/2228102/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/2228102/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "\n <linux-cifs+bounces-11108-incoming=patchwork.ozlabs.org@vger.kernel.org>", "X-Original-To": [ "incoming@patchwork.ozlabs.org", "linux-cifs@vger.kernel.org" ], "Delivered-To": "patchwork-incoming@legolas.ozlabs.org", "Authentication-Results": [ "legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256\n header.s=20251104 header.b=OxBHlE+N;\n\tdkim-atps=neutral", "legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c04:e001:36c::12fc:5321; helo=tor.lore.kernel.org;\n envelope-from=linux-cifs+bounces-11108-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)", "smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com\n header.b=\"OxBHlE+N\"", "smtp.subspace.kernel.org;\n arc=none smtp.client-ip=209.85.216.44", "smtp.subspace.kernel.org;\n dmarc=pass (p=none dis=none) header.from=gmail.com", "smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=gmail.com" ], "Received": [ "from tor.lore.kernel.org (tor.lore.kernel.org\n [IPv6:2600:3c04:e001:36c::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4g2lC70SXYz1yJ2\n\tfor <incoming@patchwork.ozlabs.org>; Sat, 25 Apr 2026 19:38:59 +1000 (AEST)", "from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby tor.lore.kernel.org (Postfix) with ESMTP id 11BC630193B5\n\tfor <incoming@patchwork.ozlabs.org>; Sat, 25 Apr 2026 09:38:53 +0000 (UTC)", "from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id BDB153876AC;\n\tSat, 25 Apr 2026 09:38:52 +0000 (UTC)", "from mail-pj1-f44.google.com (mail-pj1-f44.google.com\n [209.85.216.44])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id CD8DF38B12C\n\tfor <linux-cifs@vger.kernel.org>; Sat, 25 Apr 2026 09:38:49 +0000 (UTC)", "by mail-pj1-f44.google.com with SMTP id\n 98e67ed59e1d1-35d901060f6so1710769a91.1\n for <linux-cifs@vger.kernel.org>;\n Sat, 25 Apr 2026 02:38:49 -0700 (PDT)", "from ser8.. ([221.156.231.192])\n by smtp.gmail.com with ESMTPSA id\n d9443c01a7336-2b606ce9891sm206791725ad.83.2026.04.25.02.38.46\n (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n Sat, 25 Apr 2026 02:38:48 -0700 (PDT)" ], "ARC-Seal": "i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1777109932; cv=none;\n b=cMTrpoTCQpXX+v7S5Rjl5Avu9cpws+vRNRhUo7ksDZKdZ3X+T6LuXVFnNZpXTMiT6OTxnt10V78TLafyYgGgSnOPtJMLnLywjy23dWZlV6RTRr9mcLL5dLDfdxGYJ8WaLO8IuU/Ig8tDPa4WZr2kGwYbuPvIx3SZ7cdWgDdpF0Y=", "ARC-Message-Signature": "i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1777109932; c=relaxed/simple;\n\tbh=vLXb7ugMCCE+MqTlVuyJxG9Yf4Bgq1xnSh9pyLx2sbU=;\n\th=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References:\n\t MIME-Version;\n b=m2GR11HuZGGxxRJoNF+tddIS/Mx/drN5YQcJrpFTJ7DOIXgei7/GhW1KE6ptyD299s61UAGkrD+LeH82Oz4ExCrieCcgkFKWY4rrhUgOflZTeCRoUH183G6kd9TSSj8ezgVaonpyFADvGES8p/O/zlBcMxC1l6z4jsJB70QT9/c=", "ARC-Authentication-Results": "i=1; smtp.subspace.kernel.org;\n dmarc=pass (p=none dis=none) header.from=gmail.com;\n spf=pass smtp.mailfrom=gmail.com;\n dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com\n header.b=OxBHlE+N; arc=none smtp.client-ip=209.85.216.44", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=gmail.com; s=20251104; t=1777109929; x=1777714729;\n darn=vger.kernel.org;\n h=content-transfer-encoding:mime-version:references:in-reply-to\n :message-id:date:subject:cc:to:from:from:to:cc:subject:date\n :message-id:reply-to;\n bh=xU3Yw2OSZmM/2qmw+RTJFnRRSGp4FPsoUSqHBFDOncs=;\n b=OxBHlE+NPe776LkNam9Mh/v3lrs5EWZOGYQ5prj5H3wDBloVx7/4psp86iem/EQDs/\n kBFUfHvnJ2gQQQDB9Mor3PLvd+VITF50BbgkdVnqp7wDnql+ZfdHNZJM1M6Axf1BKVex\n 7rBhA6ZBvReRXzuH942YLdI2mjQuva8pJ9X+cIQhIyU5rC6Hb5Yvbk/YJJGq+lYE1Jg4\n jxf2T6YQIL8WQDPvtf/OQrF+IlrCJSxGlRPfkk/t/hkZUIwGH7zF2hLC8hmNroS2H5sb\n nlgmOIqMaP095TI3HFU+IAbtJrar/3WXPaIzqyUR3WVCfzF9zyGQCRaMGwfpUvlEvcY3\n SGtA==", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=1e100.net; s=20251104; t=1777109929; x=1777714729;\n h=content-transfer-encoding:mime-version:references:in-reply-to\n :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from\n :to:cc:subject:date:message-id:reply-to;\n bh=xU3Yw2OSZmM/2qmw+RTJFnRRSGp4FPsoUSqHBFDOncs=;\n b=Fc94Llw8vxszaTQ7POWWAX7+1ZuPdZJKUce3hwRyAn7X1VppjC07g5O3bEHwjWQYn7\n u+A8CWUOua3a9xpTaVn8F0QqTh5nLJ/LIAtddBkes0qRyg4bz+j5pk2ZFryyjoi6hwno\n cmJt5G6S9um1Tj50hNnp2i9whxAUJpCgW+vmV2PrAsZcHyYC4AwfnjLarm9JhHTAzI5y\n 7nMtvFgeUF7zt8W0u+4VrVdOalGNRdgzUn/P6BHA4yN0ZZ10BWY5h0xY/bA5N+yH5I8b\n CRAj8j9qIuW8P8MoJUnLMAjNFVxTjoJzUEwMSiPmGDip6d0HU82B76K4RiDQmwHC1bp4\n p99g==", "X-Forwarded-Encrypted": "i=1;\n AFNElJ8u/J2cpc5RyIt1yCyFwHGpIi6ZY3+sBko6n5WMm5tEGDxVLzbRyDckx+/AjU6275cOdg2uEO70XxG4@vger.kernel.org", "X-Gm-Message-State": "AOJu0Ywjou8KZeS/B57Tn2w4en05pKXb8XRZNz5eGuquLTCAHvIrsGq0\n\tgOMhXGy4n3Os+3Vv/NHvKoNryQIP2pFAUfchH+T+eGDpxSYydYTYs4+JKC+7JnZJ", "X-Gm-Gg": "AeBDiesIQxDz1Xt532aDOkZ6la/mWcoZXiXU+FbQzM6ujL9nfYFA76iIgdE2lHzbnLX\n\tjm37zarTcfJHLWQHlHfH3nX0a0adaFrkSJdxTTd4SSteEERqDDu3jY+uhKijuhEcQWg9CxfaS4w\n\tfV6mrEffqwXAtlmfGiSz3NmAYqrYQxGApyGjg1oDjy0ZBlwFpwsLwCEi3DAfGcX4MKVgueTCrCX\n\tdg82RWpVgf6H/nEcUczn5pzDFNq24rMqWgXTVti4ZQORDI9JZ+3Vgt3nhV3LAQ8oSAR8VucgKdZ\n\tzOVKXwzGpF17N2/YKZYMFv60nnUIlrJFz9B8dcnW4+Qt/znDADyhE/cY9Y7ZKl72YEAIKOExTC+\n\tdumignINfDW9rKz1bJ5c9vk2tVW8DacCHH3/mTyMygdYyE2CvzqmM50qsPHN4XxtWuDr8+eL7Wl\n\tJvV7P4xFz8+kAEJ4cCLGZTtwcNsBA=", "X-Received": "by 2002:a17:903:1a6b:b0:2b9:42a3:6013 with SMTP id\n d9443c01a7336-2b942a361famr13236185ad.1.1777109928798;\n Sat, 25 Apr 2026 02:38:48 -0700 (PDT)", "From": "DaeMyung Kang <charsyam@gmail.com>", "To": "Namjae Jeon <linkinjeon@kernel.org>,\n\tSteve French <smfrench@gmail.com>", "Cc": "Sergey Senozhatsky <senozhatsky@chromium.org>,\n\tTom Talpey <tom@talpey.com>,\n\tHyunchul Lee <hyc.lee@gmail.com>,\n\tRonnie Sahlberg <lsahlber@redhat.com>,\n\tlinux-cifs@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org,\n\tstable@vger.kernel.org,\n\tDaeMyung Kang <charsyam@gmail.com>", "Subject": "[PATCH 2/2] ksmbd: centralize ksmbd_conn final release to plug\n transport leak", "Date": "Sat, 25 Apr 2026 18:38:29 +0900", "Message-ID": "<20260425093829.4004785-3-charsyam@gmail.com>", "X-Mailer": "git-send-email 2.43.0", "In-Reply-To": "<20260425093829.4004785-1-charsyam@gmail.com>", "References": "<20260425093829.4004785-1-charsyam@gmail.com>", "Precedence": "bulk", "X-Mailing-List": "linux-cifs@vger.kernel.org", "List-Id": "<linux-cifs.vger.kernel.org>", "List-Subscribe": "<mailto:linux-cifs+subscribe@vger.kernel.org>", "List-Unsubscribe": "<mailto:linux-cifs+unsubscribe@vger.kernel.org>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit" }, "content": "ksmbd_conn_free() is one of four sites that can observe the last\nrefcount drop of a struct ksmbd_conn. The other three\n\n fs/smb/server/connection.c ksmbd_conn_r_count_dec()\n fs/smb/server/oplock.c __free_opinfo()\n fs/smb/server/vfs_cache.c session_fd_check()\n\nend the conn with a bare kfree(), skipping\nida_destroy(&conn->async_ida) and\nconn->transport->ops->free_transport(conn->transport). Whenever one\nof them is the last putter, the embedded async_ida and the entire\ntransport struct leak -- for TCP, that is also the struct socket and\nthe kvec iov.\n\n__free_opinfo() being a final putter is not theoretical. opinfo_put()\nqueues the callback via call_rcu(&opinfo->rcu, free_opinfo_rcu), so\nksmbd_server_terminate_conn() can deposit N opinfo releases in RCU and\nhave ksmbd_conn_free() run in the handler thread before any of them\nfire. ksmbd_conn_free() then observes refcnt > 0 and short-circuits;\nthe last RCU-delivered __free_opinfo() falls onto its bare kfree(conn)\nbranch and the transport is lost.\n\nReproducer (QEMU/virtme guest, ksmbd server and CIFS client in the\nsame guest, mounting //127.0.0.1/testshare): each iteration holds 8\nfiles open via sleep processes, force-closes TCP with\n`ss -K sport = :445`, kills the holders, lazy-umounts; repeated 10\ntimes, then ksmbd shutdown and kmemleak scan.\n\nKprobes: ksmbd_conn_alloc, ksmbd_conn_free, ksmbd_tcp_free_transport,\nfree_opinfo_rcu. (ksmbd_tcp_new_connection did not register stably\nso ksmbd_conn_alloc is the lifecycle anchor.)\n\nA/B validation, same image, varying only ksmbd.ko. Pre-patch is HEAD\nwith only patch 1 of this series applied:\n\n state conn_alloc conn_free tcp_free opi_rcu kmemleak\n ---------- ---------- --------- -------- ------- --------\n pre-patch 20 20 10 160 7\n with patch 20 20 20 160 0\n\nPre-patch conn_free=20 with tcp_free=10 directly demonstrates the\nbare-kfree paths skipping transport cleanup, and kmemleak reports 7\nunreferenced allocations whose backtraces point into the ksmbd text\n(struct tcp_transport, t->iov kvec). With this patch tcp_free matches\nconn_free at 20/20 and kmemleak is clean across two independent\npost-patch runs. opi_rcu=160 confirms the RCU opinfo release path\nthat motivates the fix is exercised. The transport leak fix is\nestablished by this A/B comparison.\n\nMove the per-struct final release into __ksmbd_conn_release_work() and\nroute the three bare-kfree final-put sites through a new\nksmbd_conn_put(). Those sites now pair ida_destroy() and\nfree_transport() with kfree(conn) regardless of which holder happens\nto release the last reference. The stop_sessions() path keeps patch\n1's local pin-drop cleanup open-coded instead of routing that\ntemporary reference through ksmbd_conn_put(), so this patch does not\nlayer the conn-put conversion on top of the stop_sessions() iteration\nrewrite and the two fixes remain independently reviewable and\nrevertible. Applying this patch alone still leaves stop_sessions()\non its own local cleanup; patch 1 fixes that site separately.\n\nThe centralized release reaches sock_release() -> tcp_close() ->\nlock_sock_nested() from every final putter. lock_sock_nested() has\nmight_sleep(), and __free_opinfo() can be released from an RCU\ncallback (free_opinfo_rcu(), softirq on default kernels). Calling\nthe final release directly from that path trips\nCONFIG_DEBUG_ATOMIC_SLEEP:\n\n BUG: sleeping function called from invalid context at net/core/sock.c:3785\n in_atomic(): 1, irqs_disabled(): 0, non_block: 0\n Call Trace:\n <IRQ>\n lock_sock_nested+0x43/0xa0\n tcp_close+0x19/0xa0\n inet_release+0x44/0x90\n sock_release+0x25/0x90\n ksmbd_tcp_free_transport+0x16/0x40 [ksmbd]\n __ksmbd_conn_release_work+0x... [ksmbd]\n ksmbd_conn_put+0x... [ksmbd]\n free_opinfo_rcu+0x... [ksmbd]\n rcu_do_batch+0x1e5/0x5c0\n rcu_core+0x395/0x4d0\n\nHandle this once inside ksmbd_conn_put() by making the final release\nunconditional through a dedicated ksmbd_conn_wq workqueue. When the\nrefcount reaches zero, ksmbd_conn_put() queues the pre-initialized\nrelease_work onto the workqueue and returns; the work handler runs\nthe sleep-allowed teardown (ida_destroy,\nfree_transport -> sock_release, kfree) in process context. That makes\nksmbd_conn_put() safe to call from RCU callbacks and other\nnon-sleeping putter contexts without each call site needing its own\nbounce.\n\nMoving the final release onto a workqueue is not by itself enough on\nthe session_fd_check() path: __close_file_table_ids() holds\nwrite_lock(&ft->lock) across the skip callback, and\nsession_fd_check() already sleeps in\nksmbd_vfs_copy_durable_owner() -> kstrdup(GFP_KERNEL) and\ndown_write(&fp->f_ci->m_lock) (a rw_semaphore) before it ever reaches\nksmbd_conn_put(). These sleeps pre-date this patch but would\nequally trip CONFIG_DEBUG_ATOMIC_SLEEP on a durable-fd workload.\nRefactor __close_file_table_ids() to take a transient reference on\nfp and unpublish fp from the session idr *under ft->lock* before\ncalling skip() outside the lock. A transient ref protects lifetime\nbut not concurrent field mutation, so the idr_remove() is what keeps\n__ksmbd_lookup_fd() through this session's idr from granting a new\nksmbd_fp_get() reference to an fp whose fp->conn / fp->tcon /\nfp->volatile_id / op->conn / lock_list links are about to be rewritten\nby session_fd_check(). The same unpublished transition also clears\nfp->volatile_id under ft->lock, preventing any later final close of\nthe same fp from removing a reused idr slot. Durable reconnect is\nunaffected because it reaches fp through the global durable table\n(ksmbd_lookup_durable_fd -> global_ft).\n\nAfter skip() returns, the preserve path drops the transient with\natomic_dec(): fp keeps the original +1 refcount that used to represent\nthe session idr entry so the durable scavenger can later expire it\nonce the timeout elapses. The close path transitions f_state to\nFP_CLOSED under ft->lock (matching ksmbd_close_fd()) so ksmbd_fp_get()\nlookups via any remaining path fail, then removes fp from m_fp_list\nbefore dropping both the transient and the original session-idr ref\nvia atomic_sub_and_test(2). The list removal cannot be left for a\ndeferred final putter because fp->volatile_id has already been cleared\nand __ksmbd_remove_fd() will intentionally skip both idr_remove() and\nlist_del_init(). The subtraction cannot underflow because no\nconcurrent close path can consume the session-idr ref after the\nidr_remove() above. If the subtraction hits zero we finalize fp\nourselves, otherwise the remaining user's ksmbd_fd_put() finalizes via\n__put_fd_final() -> __ksmbd_close_fd().\n\nThe __close_file_table_ids() refactor is exercised separately on a\ndebug kernel additionally built with CONFIG_DEBUG_LIST and\nCONFIG_DEBUG_OBJECTS_WORK using a same-session two-tcon workload:\none tcon drives an open/write storm while the other tcon repeats 50\ntree disconnects on the same session. Trace counts: 52\n__close_file_table_ids invocations, 4793 __ksmbd_close_fd calls,\n30337 __put_fd_final, 9578 ksmbd_conn_put decrements, 1\n__ksmbd_conn_release_work execution. The workload exercises the\nidr_remove() / fp->volatile_id clear / m_fp_list unlink coupling\nunder concurrent fp allocation in the same session table. This run\nvalidates the file-table/id/list rewrite under\nDEBUG_LIST/DEBUG_OBJECTS_WORK; it does not re-prove the transport\nleak fix, which the abrupt-disconnect A/B above already covered. No\nlist-corruption, work_struct ODEBUG, sleep-in-atomic, lockdep or\nkmemleak reports were observed.\n\nThe deferred-final-putter branch in __close_file_table_ids()\n(atomic_sub_and_test(2) returning false) is covered by analysis, not\nby a dedicated counter in this run: the trace points used above\ncannot distinguish a deferred-putter __ksmbd_close_fd from a normal\nSMB2_CLOSE __ksmbd_close_fd. __close_file_table_ids() unconditionally\nclears fp->volatile_id and unlinks fp from m_fp_list before\natomic_sub_and_test(2), so __ksmbd_remove_fd() invoked from a later\n__put_fd_final() correctly skips both idr_remove() and list_del_init().\n\nAt module exit, the workqueue is flushed and destroyed after\nrcu_barrier(), so any release queued by a trailing RCU callback is\ndrained before the inode hash and the module text go away. Verified\nby kprobe tracing that all 20 __ksmbd_conn_release_work() executions\ncomplete before ksmbd_conn_wq_destroy() enters, with\nksmbd_tcp_free_transport() matching ksmbd_conn_alloc() at 20/20.\n\nThe ida_destroy() previously added to ksmbd_conn_free()'s refcount\nbranch is folded into __ksmbd_conn_release_work() so it runs from\nwhichever site turns out to be the last putter.\n\nThe conversion also closes a pre-existing ksmbd_conn lifetime gap: fp\nused to store a borrowed pointer to its connection\n(fp->conn = work->conn) without taking a reference, so nothing stopped\nthe conn from being freed while an fp still held a stale fp->conn.\nTeach fp to own a strong reference on fp->conn for as long as\nfp->conn is non-NULL:\n\n * ksmbd_open_fd() and ksmbd_reopen_durable_fd() bump conn->refcnt\n when assigning fp->conn (matching put on the ksmbd_open_fd() error\n path and on the ksmbd_reopen_durable_fd() __open_id() failure\n path). Both now set fp->conn and fp->tcon before __open_id()\n publishes fp into the session's file table, so a concurrent\n teardown that iterates the table via idr cannot observe a valid\n volatile_id with fp->conn still NULL and preserve a\n partially-initialized fp.\n\n * session_fd_check() (durable preserve) and __ksmbd_close_fd() (fp\n destroy) release the owned reference via ksmbd_conn_put() and\n clear fp->conn.\n\nWith that invariant in place, session_fd_check() needs no local pin\nacross the op->conn puts -- fp's own reference keeps conn alive for\nthe entire body of the function, including the subsequent\nconn->llist_lock access. The NULL-guard at the top of\nsession_fd_check() stays: a durable reconnect that has already been\nthrough cleanup once leaves fp->conn cleared, and the lock_list loop\nwould otherwise dereference NULL.\n\nThe kernel under test was built with CONFIG_DEBUG_KMEMLEAK,\nCONFIG_PROVE_LOCKING, CONFIG_DEBUG_ATOMIC_SLEEP, CONFIG_DEBUG_OBJECTS\nand CONFIG_FAILSLAB, plus CONFIG_DEBUG_LIST and\nCONFIG_DEBUG_OBJECTS_WORK for the two-tcon stress run.\n\nThe __close_file_table_ids() refactor was also exercised under the\nsame debug kernel with a local test harness that forces\nis_reconnectable() to return true so session_fd_check() reaches the\nksmbd_vfs_copy_durable_owner()/down_write(&ci->m_lock) path for\nevery fp (Linux cifs.ko does not request durable handles in the\ngeneric mount path so the slow path is otherwise not covered). With\nthe refactor applied the harness traverses the full sleep path and\nCONFIG_DEBUG_ATOMIC_SLEEP / CONFIG_PROVE_LOCKING stay silent.\nReverting only the __close_file_table_ids() hunk while keeping the\nharness produces:\n\n BUG: sleeping function called from invalid context at vfs_cache.c:1095\n __might_sleep+0x49/0x60\n [ BUG: Invalid wait context ]\n\nwhich confirms that the refactor is what keeps ft->lock out of the\nsleepable skip() body.\n\nFixes: ee426bfb9d09 (\"ksmbd: add refcnt to ksmbd_conn struct\")\nSigned-off-by: DaeMyung Kang <charsyam@gmail.com>\n---\n fs/smb/server/connection.c | 74 ++++++++++++---\n fs/smb/server/connection.h | 5 +\n fs/smb/server/oplock.c | 4 +-\n fs/smb/server/server.c | 12 +++\n fs/smb/server/vfs_cache.c | 189 ++++++++++++++++++++++++++++++++-----\n 5 files changed, 246 insertions(+), 38 deletions(-)", "diff": "diff --git a/fs/smb/server/connection.c b/fs/smb/server/connection.c\nindex c5aac4946cbe..7438adb86fe2 100644\n--- a/fs/smb/server/connection.c\n+++ b/fs/smb/server/connection.c\n@@ -79,6 +79,62 @@ static int create_proc_clients(void) { return 0; }\n static void delete_proc_clients(void) {}\n #endif\n \n+static struct workqueue_struct *ksmbd_conn_wq;\n+\n+int ksmbd_conn_wq_init(void)\n+{\n+\tksmbd_conn_wq = alloc_workqueue(\"ksmbd-conn-release\",\n+\t\t\t\t\tWQ_UNBOUND | WQ_MEM_RECLAIM, 0);\n+\tif (!ksmbd_conn_wq)\n+\t\treturn -ENOMEM;\n+\treturn 0;\n+}\n+\n+void ksmbd_conn_wq_destroy(void)\n+{\n+\tif (ksmbd_conn_wq) {\n+\t\tdestroy_workqueue(ksmbd_conn_wq);\n+\t\tksmbd_conn_wq = NULL;\n+\t}\n+}\n+\n+/*\n+ * __ksmbd_conn_release_work() - perform the final, once-per-struct cleanup\n+ * of a ksmbd_conn whose refcount has just dropped to zero.\n+ *\n+ * This is the common release path used by ksmbd_conn_put() for the embedded\n+ * state that outlives the connection thread: async_ida and the attached\n+ * transport (which owns the socket and iov for TCP). Called from a workqueue\n+ * so that sleep-allowed teardown (sock_release -> tcp_close ->\n+ * lock_sock_nested) never runs from an RCU softirq callback (free_opinfo_rcu)\n+ * or any other non-sleeping putter context.\n+ */\n+static void __ksmbd_conn_release_work(struct work_struct *work)\n+{\n+\tstruct ksmbd_conn *conn =\n+\t\tcontainer_of(work, struct ksmbd_conn, release_work);\n+\n+\tida_destroy(&conn->async_ida);\n+\tconn->transport->ops->free_transport(conn->transport);\n+\tkfree(conn);\n+}\n+\n+/**\n+ * ksmbd_conn_put() - drop a reference and, if it was the last, queue the\n+ * release onto ksmbd_conn_wq so it runs from process context.\n+ *\n+ * Callable from any context including RCU softirq callbacks and non-sleeping\n+ * locks; the actual release is deferred to the workqueue.\n+ */\n+void ksmbd_conn_put(struct ksmbd_conn *conn)\n+{\n+\tif (!conn)\n+\t\treturn;\n+\n+\tif (atomic_dec_and_test(&conn->refcnt))\n+\t\tqueue_work(ksmbd_conn_wq, &conn->release_work);\n+}\n+\n /**\n * ksmbd_conn_free() - free resources of the connection instance\n *\n@@ -97,19 +153,7 @@ void ksmbd_conn_free(struct ksmbd_conn *conn)\n \tkvfree(conn->request_buf);\n \tkfree(conn->preauth_info);\n \tkfree(conn->mechToken);\n-\tif (atomic_dec_and_test(&conn->refcnt)) {\n-\t\t/*\n-\t\t * async_ida is embedded in struct ksmbd_conn, so pair\n-\t\t * ida_destroy() with the final kfree() rather than with\n-\t\t * the unconditional field teardown above. This keeps\n-\t\t * the IDA valid for the entire lifetime of the struct,\n-\t\t * even while other refcount holders (oplock / vfs\n-\t\t * durable handles) still reference the connection.\n-\t\t */\n-\t\tida_destroy(&conn->async_ida);\n-\t\tconn->transport->ops->free_transport(conn->transport);\n-\t\tkfree(conn);\n-\t}\n+\tksmbd_conn_put(conn);\n }\n \n /**\n@@ -136,6 +180,7 @@ struct ksmbd_conn *ksmbd_conn_alloc(void)\n \t\tconn->um = ERR_PTR(-EOPNOTSUPP);\n \tif (IS_ERR(conn->um))\n \t\tconn->um = NULL;\n+\tINIT_WORK(&conn->release_work, __ksmbd_conn_release_work);\n \tatomic_set(&conn->req_running, 0);\n \tatomic_set(&conn->r_count, 0);\n \tatomic_set(&conn->refcnt, 1);\n@@ -512,8 +557,7 @@ void ksmbd_conn_r_count_dec(struct ksmbd_conn *conn)\n \tif (!atomic_dec_return(&conn->r_count) && waitqueue_active(&conn->r_count_q))\n \t\twake_up(&conn->r_count_q);\n \n-\tif (atomic_dec_and_test(&conn->refcnt))\n-\t\tkfree(conn);\n+\tksmbd_conn_put(conn);\n }\n \n int ksmbd_conn_transport_init(void)\ndiff --git a/fs/smb/server/connection.h b/fs/smb/server/connection.h\nindex de2d46941c93..f6d38c35ea92 100644\n--- a/fs/smb/server/connection.h\n+++ b/fs/smb/server/connection.h\n@@ -16,6 +16,7 @@\n #include <linux/kthread.h>\n #include <linux/nls.h>\n #include <linux/unicode.h>\n+#include <linux/workqueue.h>\n \n #include \"smb_common.h\"\n #include \"ksmbd_work.h\"\n@@ -120,6 +121,7 @@ struct ksmbd_conn {\n \tbool\t\t\t\tbinding;\n \tatomic_t\t\t\trefcnt;\n \tbool\t\t\t\tis_aapl;\n+\tstruct work_struct\t\trelease_work;\n };\n \n struct ksmbd_conn_ops {\n@@ -164,6 +166,9 @@ void ksmbd_conn_wait_idle(struct ksmbd_conn *conn);\n int ksmbd_conn_wait_idle_sess_id(struct ksmbd_conn *curr_conn, u64 sess_id);\n struct ksmbd_conn *ksmbd_conn_alloc(void);\n void ksmbd_conn_free(struct ksmbd_conn *conn);\n+void ksmbd_conn_put(struct ksmbd_conn *conn);\n+int ksmbd_conn_wq_init(void);\n+void ksmbd_conn_wq_destroy(void);\n bool ksmbd_conn_lookup_dialect(struct ksmbd_conn *c);\n int ksmbd_conn_write(struct ksmbd_work *work);\n int ksmbd_conn_rdma_read(struct ksmbd_conn *conn,\ndiff --git a/fs/smb/server/oplock.c b/fs/smb/server/oplock.c\nindex cd3f28b0e7cb..f1e0bb650cea 100644\n--- a/fs/smb/server/oplock.c\n+++ b/fs/smb/server/oplock.c\n@@ -132,8 +132,8 @@ static void __free_opinfo(struct oplock_info *opinfo)\n {\n \tif (opinfo->is_lease)\n \t\tfree_lease(opinfo);\n-\tif (opinfo->conn && atomic_dec_and_test(&opinfo->conn->refcnt))\n-\t\tkfree(opinfo->conn);\n+\tif (opinfo->conn)\n+\t\tksmbd_conn_put(opinfo->conn);\n \tkfree(opinfo);\n }\n \ndiff --git a/fs/smb/server/server.c b/fs/smb/server/server.c\nindex 58ef02c423fc..5d799b2d4c62 100644\n--- a/fs/smb/server/server.c\n+++ b/fs/smb/server/server.c\n@@ -596,8 +596,14 @@ static int __init ksmbd_server_init(void)\n \tif (ret)\n \t\tgoto err_crypto_destroy;\n \n+\tret = ksmbd_conn_wq_init();\n+\tif (ret)\n+\t\tgoto err_workqueue_destroy;\n+\n \treturn 0;\n \n+err_workqueue_destroy:\n+\tksmbd_workqueue_destroy();\n err_crypto_destroy:\n \tksmbd_crypto_destroy();\n err_release_inode_hash:\n@@ -623,6 +629,12 @@ static void __exit ksmbd_server_exit(void)\n {\n \tksmbd_server_shutdown();\n \trcu_barrier();\n+\t/*\n+\t * ksmbd_conn_put() defers the final release onto ksmbd_conn_wq,\n+\t * so drain it after rcu_barrier() has fired any pending RCU\n+\t * callbacks that may have queued a release.\n+\t */\n+\tksmbd_conn_wq_destroy();\n \tksmbd_release_inode_hash();\n }\n \ndiff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c\nindex 3551f01a3fa0..b7cc4f1bc021 100644\n--- a/fs/smb/server/vfs_cache.c\n+++ b/fs/smb/server/vfs_cache.c\n@@ -475,6 +475,17 @@ static void __ksmbd_close_fd(struct ksmbd_file_table *ft, struct ksmbd_file *fp)\n \t\tkfree(smb_lock);\n \t}\n \n+\t/*\n+\t * Drop fp's strong reference on conn (taken in ksmbd_open_fd() /\n+\t * ksmbd_reopen_durable_fd()). Durable fps that reached the\n+\t * scavenger have already had fp->conn cleared by session_fd_check(),\n+\t * in which case there is nothing to drop here.\n+\t */\n+\tif (fp->conn) {\n+\t\tksmbd_conn_put(fp->conn);\n+\t\tfp->conn = NULL;\n+\t}\n+\n \tif (ksmbd_stream_fd(fp))\n \t\tkfree(fp->stream.name);\n \tkfree(fp->owner.name);\n@@ -752,6 +763,14 @@ struct ksmbd_file *ksmbd_open_fd(struct ksmbd_work *work, struct file *filp)\n \tatomic_set(&fp->refcount, 1);\n \n \tfp->filp\t\t= filp;\n+\t/*\n+\t * fp owns a strong reference on fp->conn for as long as fp->conn is\n+\t * non-NULL, so session_fd_check() and __ksmbd_close_fd() never\n+\t * dereference a dangling pointer. Paired with ksmbd_conn_put() in\n+\t * session_fd_check() (durable preserve), in __ksmbd_close_fd()\n+\t * (final close), and on the error paths below.\n+\t */\n+\tatomic_inc(&work->conn->refcnt);\n \tfp->conn\t\t= work->conn;\n \tfp->tcon\t\t= work->tcon;\n \tfp->volatile_id\t\t= KSMBD_NO_FID;\n@@ -774,6 +793,9 @@ struct ksmbd_file *ksmbd_open_fd(struct ksmbd_work *work, struct file *filp)\n \treturn fp;\n \n err_out:\n+\t/* fp->conn was set and refcounted before every branch here. */\n+\tksmbd_conn_put(fp->conn);\n+\tfp->conn = NULL;\n \tkmem_cache_free(filp_cache, fp);\n \treturn ERR_PTR(ret);\n }\n@@ -794,7 +816,8 @@ __close_file_table_ids(struct ksmbd_session *sess,\n \t\t struct ksmbd_tree_connect *tcon,\n \t\t bool (*skip)(struct ksmbd_tree_connect *tcon,\n \t\t\t\t struct ksmbd_file *fp,\n-\t\t\t\t struct ksmbd_user *user))\n+\t\t\t\t struct ksmbd_user *user),\n+\t\t bool skip_preserves_fp)\n {\n \tstruct ksmbd_file_table *ft = &sess->file_table;\n \tstruct ksmbd_file *fp;\n@@ -802,32 +825,132 @@ __close_file_table_ids(struct ksmbd_session *sess,\n \tint num = 0;\n \n \twhile (1) {\n+\t\tunsigned int saved_id;\n+\n \t\twrite_lock(&ft->lock);\n \t\tfp = idr_get_next(ft->idr, &id);\n \t\tif (!fp) {\n \t\t\twrite_unlock(&ft->lock);\n \t\t\tbreak;\n \t\t}\n-\n-\t\tif (skip(tcon, fp, sess->user) ||\n-\t\t !atomic_dec_and_test(&fp->refcount)) {\n+\t\tsaved_id = id;\n+\t\tif (!atomic_inc_not_zero(&fp->refcount)) {\n \t\t\tid++;\n \t\t\twrite_unlock(&ft->lock);\n \t\t\tcontinue;\n \t\t}\n \n-\t\tset_close_state_blocked_works(fp);\n-\t\tidr_remove(ft->idr, fp->volatile_id);\n-\t\tfp->volatile_id = KSMBD_NO_FID;\n-\t\twrite_unlock(&ft->lock);\n+\t\tif (skip_preserves_fp) {\n+\t\t\t/*\n+\t\t\t * Session teardown: skip() is session_fd_check(),\n+\t\t\t * which may sleep and mutates fp->conn / fp->tcon /\n+\t\t\t * fp->volatile_id when it chooses to preserve fp\n+\t\t\t * for durable reconnect. Unpublish fp from the\n+\t\t\t * session idr here, under ft->lock, so that neither\n+\t\t\t * __ksmbd_lookup_fd() nor a concurrent\n+\t\t\t * ksmbd_close_fd() through this session can reach\n+\t\t\t * fp -- the former would otherwise grant a new\n+\t\t\t * ksmbd_fp_get() reference to an fp whose fields\n+\t\t\t * are about to be rewritten outside the lock, and\n+\t\t\t * the latter would otherwise consume the original\n+\t\t\t * idr-owned ref out from under the\n+\t\t\t * atomic_sub_and_test(2) below. Durable reconnect\n+\t\t\t * still reaches fp via global_ft.\n+\t\t\t */\n+\t\t\tidr_remove(ft->idr, saved_id);\n+\t\t\tfp->volatile_id = KSMBD_NO_FID;\n+\t\t\twrite_unlock(&ft->lock);\n \n+\t\t\tif (skip(tcon, fp, sess->user)) {\n+\t\t\t\t/*\n+\t\t\t\t * session_fd_check() has converted fp to\n+\t\t\t\t * durable-preserve state and cleared its\n+\t\t\t\t * per-conn fields. fp is already unpublished\n+\t\t\t\t * above; the original idr-owned ref keeps it\n+\t\t\t\t * alive for the durable scavenger. Drop only\n+\t\t\t\t * the transient ref. atomic_dec() is safe --\n+\t\t\t\t * atomic_inc_not_zero() succeeded on a\n+\t\t\t\t * positive value and we added one more, so\n+\t\t\t\t * refcount cannot be zero here.\n+\t\t\t\t */\n+\t\t\t\tatomic_dec(&fp->refcount);\n+\t\t\t\tid++;\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n+\t\t\t/*\n+\t\t\t * Close path. fp is already out of the session\n+\t\t\t * idr, so no concurrent close path can consume the\n+\t\t\t * idr-owned ref before our atomic_sub_and_test(2)\n+\t\t\t * below. Block any remaining lookup (global_ft,\n+\t\t\t * m_fp_list) by transitioning f_state to FP_CLOSED;\n+\t\t\t * ksmbd_fp_get() refuses non-FP_INITED files.\n+\t\t\t */\n+\t\t\twrite_lock(&ft->lock);\n+\t\t\tif (fp->f_state == FP_INITED) {\n+\t\t\t\tset_close_state_blocked_works(fp);\n+\t\t\t\tfp->f_state = FP_CLOSED;\n+\t\t\t}\n+\t\t\twrite_unlock(&ft->lock);\n+\t\t} else {\n+\t\t\t/*\n+\t\t\t * Tree teardown: skip() is tree_conn_fd_check(), a\n+\t\t\t * cheap pointer compare that doesn't sleep and has\n+\t\t\t * no side effects, so keep the skip decision plus\n+\t\t\t * the unpublish-and-mark-closed sequence atomic\n+\t\t\t * under ft->lock. fps belonging to other tree\n+\t\t\t * connects (skip() == true) stay fully published in\n+\t\t\t * the session idr with no lock window; fps we\n+\t\t\t * intend to close are unpublished and transitioned\n+\t\t\t * to FP_CLOSED before we release the lock, so no\n+\t\t\t * concurrent ksmbd_close_fd() can consume the\n+\t\t\t * idr-owned ref before our atomic_sub_and_test(2)\n+\t\t\t * below.\n+\t\t\t */\n+\t\t\tif (skip(tcon, fp, sess->user)) {\n+\t\t\t\tatomic_dec(&fp->refcount);\n+\t\t\t\twrite_unlock(&ft->lock);\n+\t\t\t\tid++;\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\tidr_remove(ft->idr, saved_id);\n+\t\t\tfp->volatile_id = KSMBD_NO_FID;\n+\t\t\tif (fp->f_state == FP_INITED) {\n+\t\t\t\tset_close_state_blocked_works(fp);\n+\t\t\t\tfp->f_state = FP_CLOSED;\n+\t\t\t}\n+\t\t\twrite_unlock(&ft->lock);\n+\t\t}\n+\n+\t\t/*\n+\t\t * fp->volatile_id is already cleared to prevent stale idr\n+\t\t * removal from a deferred final close. Remove fp from\n+\t\t * m_fp_list here because __ksmbd_remove_fd() will skip the\n+\t\t * list unlink when volatile_id is KSMBD_NO_FID.\n+\t\t */\n \t\tdown_write(&fp->f_ci->m_lock);\n \t\tlist_del_init(&fp->node);\n \t\tup_write(&fp->f_ci->m_lock);\n \n-\t\t__ksmbd_close_fd(ft, fp);\n-\n-\t\tnum++;\n+\t\t/*\n+\t\t * Drop both our transient (+1) and the original\n+\t\t * session-idr-owned ref via atomic_sub_and_test(2). By\n+\t\t * the reasoning in each branch above, no concurrent close\n+\t\t * path can consume the idr-owned ref, so the subtraction\n+\t\t * cannot underflow.\n+\t\t *\n+\t\t * If we end up as the final putter, finalize fp and\n+\t\t * account the open_files_count decrement via the caller's\n+\t\t * atomic_sub(num, ...). Otherwise the remaining user's\n+\t\t * ksmbd_fd_put() reaches __put_fd_final(), which does its\n+\t\t * own atomic_dec(&open_files_count), so we must not count\n+\t\t * this fp here -- doing so would double-decrement the\n+\t\t * connection-wide counter.\n+\t\t */\n+\t\tif (atomic_sub_and_test(2, &fp->refcount)) {\n+\t\t\t__ksmbd_close_fd(NULL, fp);\n+\t\t\tnum++;\n+\t\t}\n \t\tid++;\n \t}\n \n@@ -1062,25 +1185,32 @@ static bool session_fd_check(struct ksmbd_tree_connect *tcon,\n \tif (!is_reconnectable(fp))\n \t\treturn false;\n \n+\tif (!fp->conn)\n+\t\treturn true;\n+\n \tif (ksmbd_vfs_copy_durable_owner(fp, user))\n \t\treturn false;\n \n+\t/*\n+\t * fp owns a strong reference on fp->conn (taken in ksmbd_open_fd()\n+\t * / ksmbd_reopen_durable_fd()), so conn stays valid for the whole\n+\t * body of this function regardless of any op->conn puts below.\n+\t */\n \tconn = fp->conn;\n \tci = fp->f_ci;\n \tdown_write(&ci->m_lock);\n \tlist_for_each_entry_rcu(op, &ci->m_op_list, op_entry) {\n \t\tif (op->conn != conn)\n \t\t\tcontinue;\n-\t\tif (op->conn && atomic_dec_and_test(&op->conn->refcnt))\n-\t\t\tkfree(op->conn);\n+\t\tksmbd_conn_put(op->conn);\n \t\top->conn = NULL;\n \t}\n \tup_write(&ci->m_lock);\n \n \tlist_for_each_entry_safe(smb_lock, tmp_lock, &fp->lock_list, flist) {\n-\t\tspin_lock(&fp->conn->llist_lock);\n+\t\tspin_lock(&conn->llist_lock);\n \t\tlist_del_init(&smb_lock->clist);\n-\t\tspin_unlock(&fp->conn->llist_lock);\n+\t\tspin_unlock(&conn->llist_lock);\n \t}\n \n \tfp->conn = NULL;\n@@ -1091,6 +1221,8 @@ static bool session_fd_check(struct ksmbd_tree_connect *tcon,\n \t\tfp->durable_scavenger_timeout =\n \t\t\tjiffies_to_msecs(jiffies) + fp->durable_timeout;\n \n+\t/* Drop fp's own reference on conn. */\n+\tksmbd_conn_put(conn);\n \treturn true;\n }\n \n@@ -1098,7 +1230,8 @@ void ksmbd_close_tree_conn_fds(struct ksmbd_work *work)\n {\n \tint num = __close_file_table_ids(work->sess,\n \t\t\t\t\t work->tcon,\n-\t\t\t\t\t tree_conn_fd_check);\n+\t\t\t\t\t tree_conn_fd_check,\n+\t\t\t\t\t false);\n \n \tatomic_sub(num, &work->conn->stats.open_files_count);\n }\n@@ -1107,7 +1240,8 @@ void ksmbd_close_session_fds(struct ksmbd_work *work)\n {\n \tint num = __close_file_table_ids(work->sess,\n \t\t\t\t\t work->tcon,\n-\t\t\t\t\t session_fd_check);\n+\t\t\t\t\t session_fd_check,\n+\t\t\t\t\t true);\n \n \tatomic_sub(num, &work->conn->stats.open_files_count);\n }\n@@ -1178,15 +1312,28 @@ int ksmbd_reopen_durable_fd(struct ksmbd_work *work, struct ksmbd_file *fp)\n \n \told_f_state = fp->f_state;\n \tfp->f_state = FP_NEW;\n+\n+\t/*\n+\t * Initialize fp's connection binding before publishing fp into the\n+\t * session's file table. If __open_id() is ordered first, a\n+\t * concurrent teardown that iterates the table can observe a valid\n+\t * volatile_id with fp->conn == NULL and preserve a\n+\t * partially-initialized fp. fp owns a strong reference on the new\n+\t * conn (see ksmbd_open_fd()); undo it on __open_id() failure.\n+\t */\n+\tatomic_inc(&conn->refcnt);\n+\tfp->conn = conn;\n+\tfp->tcon = work->tcon;\n+\n \t__open_id(&work->sess->file_table, fp, OPEN_ID_TYPE_VOLATILE_ID);\n \tif (!has_file_id(fp->volatile_id)) {\n+\t\tfp->conn = NULL;\n+\t\tfp->tcon = NULL;\n+\t\tksmbd_conn_put(conn);\n \t\tfp->f_state = old_f_state;\n \t\treturn -EBADF;\n \t}\n \n-\tfp->conn = conn;\n-\tfp->tcon = work->tcon;\n-\n \tlist_for_each_entry(smb_lock, &fp->lock_list, flist) {\n \t\tspin_lock(&conn->llist_lock);\n \t\tlist_add_tail(&smb_lock->clist, &conn->lock_list);\n@@ -1228,7 +1375,7 @@ void ksmbd_destroy_file_table(struct ksmbd_session *sess)\n \tif (!ft->idr)\n \t\treturn;\n \n-\t__close_file_table_ids(sess, NULL, session_fd_check);\n+\t__close_file_table_ids(sess, NULL, session_fd_check, true);\n \tidr_destroy(ft->idr);\n \tkfree(ft->idr);\n \tft->idr = NULL;\n", "prefixes": [ "2/2" ] }