From nobody Tue Dec 2 00:46:00 2025 Received: from mail-pg1-f172.google.com (mail-pg1-f172.google.com [209.85.215.172]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5CDFE2135D7 for ; Sun, 23 Nov 2025 02:54:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.172 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763866468; cv=none; b=E4uF2eOBNSgsFvNOiAtL8j5QCkhlT3C7XZlCMmW+PV/DZuZWARjpUIJpAyVoEhHWcxy87IeakUzyWHgUDx3RCWKGIUbRbr/NmmIJJB+Y1UCeLQlsEidE3jPXnn87n2/Dwy5k96VLEbZzRhaDS58VD1tPavtQY9Ychc+IG+l5ff4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763866468; c=relaxed/simple; bh=VPSEhOnB2RE6eQydB0Smr96MaVFBkvWgt2EDOEnbyuA=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=mUb5t2zBOioN/LPv7z0+dNJVQC7VipCNj0PhIYUyiSQLUIGQ4uPMzgezkB9QSVVUXIHLV0qwUwqODymDQQX+ciE+CROAfCwcD9ddJUp+eciJPafTChE7D6CF+mkLYCQ+qB8cvN+bfYI1GuWXkaVZgjZWwj42VqGazzU61hdxxwI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=Ru2Kf5ta; arc=none smtp.client-ip=209.85.215.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Ru2Kf5ta" Received: by mail-pg1-f172.google.com with SMTP id 41be03b00d2f7-bceaaed0514so261798a12.3 for ; Sat, 22 Nov 2025 18:54:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1763866464; x=1764471264; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=+rWyDuox+fsgvKye3HBmJxKH4MrEcdZHrSZHWuNczAU=; b=Ru2Kf5taPfYQ76Wzf60aQ1WFd4/4hmtFhaFR4sWgTn+4DLVmceCuRBZZZgx+hFe0lr kFdD3SB2rV2uXwgrTLfn7tVzzjJPrxyPhhh+4cKAPKCK6jIO8OjvBftVf9VkyW+pbIdc 9uPxsky22hcfz3TP+CSpP1ztoJB4/P3XNgYFzlnxAijH1RNi1MG/BHim2e5VyBu2ohN7 gt6siTEHi7ChlBR1usICkfmmR82o8Sz0ekY281FmFbF/jLx3asoOU8JIvzV/Dd4J6CL1 6m8I6+zuO7mm8vKuP+ARnallH5rDm1PiyZ5/gyP6kIsC/PEIj9IaSbJF5ZjhIylCj+K1 kI5A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763866464; x=1764471264; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=+rWyDuox+fsgvKye3HBmJxKH4MrEcdZHrSZHWuNczAU=; b=elXdLPPIcmwZXqBKsDHNWHIhBLQJ1OERdaubo2qmqL9z+WX9oKHOIZYqAb/7zpsnJY 33aanBkFzqBvnVrducdEPAZ4GVXOPiJO5HlCQ4gp7xmSY7O5stnSvS/xmD86j7oDraqm wgx3tZElIbkF/em/mhsIjgT1Y16f1st+X4abrvLKmcGHoBmTHqe8/ECjjiLxo7vT1FE+ rVFWfSlw0lovnuphNDCCoF/1JM86LhGJYj1xm/PpBOspnJo74M3IsBJuVVkBNrNsXcLd Qo/OGrZHl4e6hTbHLHYK0bkLQJnldBk24/L232KSmueZRWyWtFMk83UVqFLnXBtSWkg7 S7hA== X-Forwarded-Encrypted: i=1; AJvYcCWsYDAfPNO80rH8rpVyDwmfmrXAzkeDQHqrP37fHtr3W9djsGbctp7bbvEisP+vyb5DaQVePM3jq8yROts=@vger.kernel.org X-Gm-Message-State: AOJu0YwfZh9qXQ1z63PkoxPYGptLLpsCSU8pMaySu2r+CgiIgqPHTWFC BxVqMH1yTfJAL/5Gt4yHL1TdjNkaxjk3f6UnWj3YwITEtDFE0EepZWaO X-Gm-Gg: ASbGncvF3m8L0NCI1owDX4rgi7r5hU2p9wdVK+zAXdGQhRNtyW2HuP95CK54TXHsBCx XNT0zU9o4Ri//mbTNTyVmOP4ixQhwpRhP6jxk6vzPnj5mjzS2FfowVxB1COM6iPBbBhRK6/b+S2 FO44CpSmis52rOI8Uc25nXGCz3r4Z207tn01ljiJ8QvIVJoQgeba+jPDkmiANXVuqBeO3obGciw ScEA1Xb+mZSscgK41K96PmnjnJSboXgFbkHODqsl+N5SoJTcQI9K0XE0ofzJ3YrzduOTtLZ7Sye HT/7OxOm0Thjrrk4FBW5xEbMp+o4BBJUZuU8kU9XhOrAZ2S2pTXo6vN1LlSUwPpFlyhlsK9jrPW e5Wxp/RFDa/p8L4d4zBhtz1l20SGLINuz56sNw/UbeIM83HwTUJXReqZMgEEmoGObdmqD28k0Ew DX8ZJpxxElIc8c1nthhPFq9D8OjobqLX94ZYfkctquxEqXZNSxfE1wLnozXNU/BWM4PhAAHkG+ X-Google-Smtp-Source: AGHT+IEOwgcBpDEpwRs7NjxIYRpA59LU4lAa4Z5co3ra/2xjeKkaDavEtEeAipX/NV+VPLbSZ81R9Q== X-Received: by 2002:a17:90b:3e8b:b0:340:e0f3:8212 with SMTP id 98e67ed59e1d1-34736bdad39mr4096260a91.8.1763866464426; Sat, 22 Nov 2025 18:54:24 -0800 (PST) Received: from poi.localdomain (KD118158218050.ppp-bb.dion.ne.jp. [118.158.218.50]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-345b03e04e5sm8281266a91.6.2025.11.22.18.54.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 22 Nov 2025 18:54:23 -0800 (PST) From: Qianchang Zhao To: Namjae Jeon , Steve French Cc: gregkh@linuxfoundation.org, linux-cifs@vger.kernel.org, linux-kernel@vger.kernel.org, Zhitong Liu , Qianchang Zhao , stable@vger.kernel.org Subject: [PATCH] ksmbd: vfs: fix race on m_flags in vfs_cache Date: Sun, 23 Nov 2025 11:54:14 +0900 Message-Id: <20251123025414.644641-1-pioooooooooip@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" ksmbd maintains delete-on-close and pending-delete state in ksmbd_inode->m_flags. In vfs_cache.c this field is accessed under inconsistent locking: some paths read and modify m_flags under ci->m_lock while others do so without taking the lock at all. Examples: - ksmbd_query_inode_status() and __ksmbd_inode_close() use ci->m_lock when checking or updating m_flags. - ksmbd_inode_pending_delete(), ksmbd_set_inode_pending_delete(), ksmbd_clear_inode_pending_delete() and ksmbd_fd_set_delete_on_close() used to read and modify m_flags without ci->m_lock. This creates a potential data race on m_flags when multiple threads open, close and delete the same file concurrently. In the worst case delete-on-close and pending-delete bits can be lost or observed in an inconsistent state, leading to confusing delete semantics (files that stay on disk after delete-on-close, or files that disappear while still in use). Fix it by: - Making ksmbd_query_inode_status() look at m_flags under ci->m_lock after dropping inode_hash_lock. - Adding ci->m_lock protection to all helpers that read or modify m_flags (ksmbd_inode_pending_delete(), ksmbd_set_inode_pending_delete(), ksmbd_clear_inode_pending_delete(), ksmbd_fd_set_delete_on_close()). - Keeping the existing ci->m_lock protection in __ksmbd_inode_close(), and moving the actual unlink/xattr removal outside the lock. This unifies the locking around m_flags and removes the data race while preserving the existing delete-on-close behaviour. Reported-by: Qianchang Zhao Reported-by: Zhitong Liu Cc: stable@vger.kernel.org Signed-off-by: Qianchang Zhao --- fs/smb/server/vfs_cache.c | 114 +++++++++++++++++++++++++++++--------- 1 file changed, 87 insertions(+), 27 deletions(-) diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c index dfed6fce8..b44e0d618 100644 --- a/fs/smb/server/vfs_cache.c +++ b/fs/smb/server/vfs_cache.c @@ -112,40 +112,88 @@ int ksmbd_query_inode_status(struct dentry *dentry) =20 read_lock(&inode_hash_lock); ci =3D __ksmbd_inode_lookup(dentry); - if (ci) { - ret =3D KSMBD_INODE_STATUS_OK; - if (ci->m_flags & (S_DEL_PENDING | S_DEL_ON_CLS)) - ret =3D KSMBD_INODE_STATUS_PENDING_DELETE; - atomic_dec(&ci->m_count); - } read_unlock(&inode_hash_lock); + if (!ci) + return ret; + down_read(&ci->m_lock); + if (ci->m_flags & (S_DEL_PENDING | S_DEL_ON_CLS)) + ret =3D KSMBD_INODE_STATUS_PENDING_DELETE; + else + ret =3D KSMBD_INODE_STATUS_OK; + up_read(&ci->m_lock); + + atomic_dec(&ci->m_count); return ret; } =20 bool ksmbd_inode_pending_delete(struct ksmbd_file *fp) { - return (fp->f_ci->m_flags & (S_DEL_PENDING | S_DEL_ON_CLS)); + struct ksmbd_inode *ci; + bool ret; + + if (!fp) + return false; + + ci =3D fp->f_ci; + if (!ci) + return false; + + down_read(&ci->m_lock); + ret =3D ci->m_flags & (S_DEL_PENDING | S_DEL_ON_CLS); + up_read(&ci->m_lock); + + return ret; } =20 void ksmbd_set_inode_pending_delete(struct ksmbd_file *fp) { - fp->f_ci->m_flags |=3D S_DEL_PENDING; + struct ksmbd_inode *ci; + + if (!fp) + return; + + ci =3D fp->f_ci; + if (!ci) + return; + + down_write(&ci->m_lock); + ci->m_flags |=3D S_DEL_PENDING; + up_write(&ci->m_lock); } =20 void ksmbd_clear_inode_pending_delete(struct ksmbd_file *fp) { - fp->f_ci->m_flags &=3D ~S_DEL_PENDING; + struct ksmbd_inode *ci; + + if (!fp) + return; + + ci =3D fp->f_ci; + if (!ci) + return; + + down_write(&ci->m_lock); + ci->m_flags &=3D ~S_DEL_PENDING; + up_write(&ci->m_lock); } =20 -void ksmbd_fd_set_delete_on_close(struct ksmbd_file *fp, - int file_info) +void ksmbd_fd_set_delete_on_close(struct ksmbd_file *fp, int file_info) { - if (ksmbd_stream_fd(fp)) { - fp->f_ci->m_flags |=3D S_DEL_ON_CLS_STREAM; + struct ksmbd_inode *ci; + + if (!fp) + return; + + ci =3D fp->f_ci; + if (!ci) return; - } =20 - fp->f_ci->m_flags |=3D S_DEL_ON_CLS; + down_write(&ci->m_lock); + if (ksmbd_stream_fd(fp)) + ci->m_flags |=3D S_DEL_ON_CLS_STREAM; + else + ci->m_flags |=3D S_DEL_ON_CLS; + up_write(&ci->m_lock); } =20 static void ksmbd_inode_hash(struct ksmbd_inode *ci) @@ -255,29 +303,41 @@ static void __ksmbd_inode_close(struct ksmbd_file *fp) struct ksmbd_inode *ci =3D fp->f_ci; int err; struct file *filp; + bool remove_stream_xattr =3D false; + bool do_unlink =3D false; =20 filp =3D fp->filp; - if (ksmbd_stream_fd(fp) && (ci->m_flags & S_DEL_ON_CLS_STREAM)) { - ci->m_flags &=3D ~S_DEL_ON_CLS_STREAM; - err =3D ksmbd_vfs_remove_xattr(file_mnt_idmap(filp), - &filp->f_path, - fp->stream.name, - true); - if (err) - pr_err("remove xattr failed : %s\n", - fp->stream.name); + + if (ksmbd_stream_fd(fp)) { + down_write(&ci->m_lock); + if (ci->m_flags & S_DEL_ON_CLS_STREAM) { + ci->m_flags &=3D ~S_DEL_ON_CLS_STREAM; + remove_stream_xattr =3D true; + } + up_write(&ci->m_lock); + + if (remove_stream_xattr) { + err =3D ksmbd_vfs_remove_xattr(file_mnt_idmap(filp), + &filp->f_path, + fp->stream.name, + true); + if (err) + pr_err("remove xattr failed : %s\n", + fp->stream.name); + } } =20 if (atomic_dec_and_test(&ci->m_count)) { down_write(&ci->m_lock); if (ci->m_flags & (S_DEL_ON_CLS | S_DEL_PENDING)) { ci->m_flags &=3D ~(S_DEL_ON_CLS | S_DEL_PENDING); - up_write(&ci->m_lock); - ksmbd_vfs_unlink(filp); - down_write(&ci->m_lock); + do_unlink =3D true; } up_write(&ci->m_lock); =20 + if (do_unlink) + ksmbd_vfs_unlink(filp); + ksmbd_inode_free(ci); } } --=20 2.34.1