From patchwork Wed Mar 6 03:43:51 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Meetakshi Setiya X-Patchwork-Id: 13583317 Received: from mail-oi1-f182.google.com (mail-oi1-f182.google.com [209.85.167.182]) (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 45302173; Wed, 6 Mar 2024 03:43:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.182 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709696641; cv=none; b=oKJmNoua5BvCfFixxY6pAS6VxIS2BBaorqWfk1pB9obkf0UCsHxyh3vQhOZt+xQTyzhlCnFUp3nfq8eKr4UimEeeWyN5o8CLrcuagzryMQoELYvgNzFRsUoJlKCuA0ndAi16Zwy17PXP25eCLQPzeAj04M6/JbPFj1qbS90bFFw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709696641; c=relaxed/simple; bh=rREKfGyeLsi7U8KWXmPSj4cPErDPLaYLM31aROKAF+I=; h=From:To:Cc:Subject:Date:Message-Id:MIME-Version; b=VbHnZ9WA/dLDAVC2jXO5zrvF0buZADfQDPxZtxroJz1Wyi/AJfBOm7CTX7zQPZCPIndxtk0Adaw8QUrUG2K4o6v7xQQuSzjOpCPkrc86oS+nMjUX1qAUPlXggdy9U/N6jfEy6vgnedZ6PzvERn1l2Nk4iHpdyd8/6UT279/twnk= 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=mNpE05pp; arc=none smtp.client-ip=209.85.167.182 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="mNpE05pp" Received: by mail-oi1-f182.google.com with SMTP id 5614622812f47-3c1e992f069so192744b6e.3; Tue, 05 Mar 2024 19:43:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1709696638; x=1710301438; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=Ow0Cmf62S6CGs5TLbMKI23uaJJvz87r8Z80q04Fa77M=; b=mNpE05ppIJKL6K8OBrxbPogTbYlN8DMSt7wBH/YVxvAMePJP62O7rec4VedCYsh18O iGJprhjTPtrXHx8DPvQGHO9WDCNz4wazIWvGauA8rtBrTOWOWckLwVm5ATKNFKCya9qP AfbwO5+S6Oj1xwDfkdh7gfo2rMVADXDV2mKFVCpq+dJe3yZEyV3vRpJzTQN+cxFZR1Hs bcUV+4NHKvksa4z3RKgJXWp/PjSkRHOcrLQwS85efc89/Z0uLV9tmQCywJlvhDk7mEEH 274yasmNeqYqHuhcRlQVUCGtK8Cxk4FZOeVNgh5Q4uXlgCARbhrmQVLAV6/7NJL9Ysfv 7g5A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709696638; x=1710301438; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Ow0Cmf62S6CGs5TLbMKI23uaJJvz87r8Z80q04Fa77M=; b=YrDmHV7wzk/qZzfztY6aJ+0v4alDvZRqH9xxvXbxn5qo3DnRiJy/y7zpgCwAibT4kW LhAbRQTesmrlapIwO/ZkBChiJ1Bakh1bJZ9ZAlYUmQYo1FCuD/brz25iAvu6Q2OUtPYG iIIwQjYYo7D8RUvB1kzoYBGFp34mAHjrvyCt1wCD69xCyPfDgkied00FAUhUgpeKkck9 d3YpRhJNVcNdz8fHG6CczGdcsoaQsUs5oDidWg7v0p6PkidfY8qay56JOJunuZ7ClTit ZTx0i6OJU4HMH2lXzlx0ZbIrYfygkXrYPRvCrL6wGGglWgwFFM2u1AfynS+rWOful8uz jSVA== X-Forwarded-Encrypted: i=1; AJvYcCUBA+K9ohOmzKJwcKeBUwe/Dco/WuHKuqy2uyayLz179xSvBh3AanWEQ0LGHUwOgxQ0zW5ZIBPDB1y8a8YjCih/UJ+8olX9eNsnu3rby0X8Rus7eQ7c1Gasjpsb1qoqNy2MD0ZL3Te/rQ== X-Gm-Message-State: AOJu0YzfOtJHGUNOMa7bicfKkKEV4R5Q7geZzwnfd33699Ll+P2wRVlD hFjovCLYF4xZJfpGjue8FInJmNoasRC+bJAkuzI5vWIEGeTJqTdERWVWNDC2nNs= X-Google-Smtp-Source: AGHT+IE3umW5vIsnWK9VAzOkRzZs+Z2PMvGU29XhXNO3Q6p++HuaumPjxCKTo0KRArVGTCsmgM9Kkg== X-Received: by 2002:aca:f1a:0:b0:3c1:f531:7e9b with SMTP id 26-20020aca0f1a000000b003c1f5317e9bmr3222149oip.27.1709696638281; Tue, 05 Mar 2024 19:43:58 -0800 (PST) Received: from met-Virtual-Machine.. ([131.107.159.31]) by smtp.gmail.com with ESMTPSA id j5-20020a654305000000b005db034d1514sm8639808pgq.82.2024.03.05.19.43.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 Mar 2024 19:43:57 -0800 (PST) From: meetakshisetiyaoss@gmail.com To: sfrench@samba.org, pc@manguebit.com, ronniesahlberg@gmail.com, sprasad@microsoft.com, nspmangalore@gmail.com, tom@talpey.com, linux-cifs@vger.kernel.org, linux-kernel@vger.kernel.org, samba-technical@lists.samba.org, bharathsm.hsk@gmail.com Cc: Meetakshi Setiya Subject: [PATCH 1/3] smb: client: reuse file lease key in compound operations Date: Tue, 5 Mar 2024 22:43:51 -0500 Message-Id: <20240306034353.190039-1-meetakshisetiyaoss@gmail.com> X-Mailer: git-send-email 2.39.2 Precedence: bulk X-Mailing-List: linux-cifs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Meetakshi Setiya Currently, when a rename, unlink or set path size compound operation is requested on a file that has a lot of dirty pages to be written to the server, we do not send the lease key for these requests. As a result, the server can assume that this request is from a new client, and send a lease break notification to the same client, on the same connection. As a response to the lease break, the client can consume several credits to write the dirty pages to the server. Depending on the server's credit grant implementation, the server can stop granting more credits to this connection, and this can cause a deadlock (which can only be resolved when the lease timer on the server expires). One of the problems here is that the client is sending no lease key, even if it has a lease for the file. This patch fixes the problem by reusing the existing lease key on the file for rename, unlink and set path size compound operations so that the client does not break its own lease. A very trivial example could be a set of commands by a client that maintains open handle (for write) to a file and then tries to copy the contents of that file to another one, eg., tail -f /dev/null > myfile & mv myfile myfile2 Presently, the network capture on the client shows that the move (or rename) would trigger a lease break on the same client, for the same file. With the lease key reused, the lease break request-response overhead is eliminated, thereby reducing the roundtrips performed for this set of operations. The patch fixes the bug described above and also provides perf benefit. Signed-off-by: Meetakshi Setiya --- fs/smb/client/cifsglob.h | 5 ++-- fs/smb/client/cifsproto.h | 6 +++-- fs/smb/client/cifssmb.c | 4 ++-- fs/smb/client/inode.c | 10 ++++---- fs/smb/client/smb2inode.c | 48 ++++++++++++++++++++++++--------------- fs/smb/client/smb2proto.h | 6 +++-- 6 files changed, 48 insertions(+), 31 deletions(-) diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h index 16befff4cbb4..50f7e47c2229 100644 --- a/fs/smb/client/cifsglob.h +++ b/fs/smb/client/cifsglob.h @@ -371,7 +371,8 @@ struct smb_version_operations { struct cifs_open_info_data *data); /* set size by path */ int (*set_path_size)(const unsigned int, struct cifs_tcon *, - const char *, __u64, struct cifs_sb_info *, bool); + const char *, __u64, struct cifs_sb_info *, bool, + struct dentry *); /* set size by file handle */ int (*set_file_size)(const unsigned int, struct cifs_tcon *, struct cifsFileInfo *, __u64, bool); @@ -401,7 +402,7 @@ struct smb_version_operations { struct cifs_sb_info *); /* unlink file */ int (*unlink)(const unsigned int, struct cifs_tcon *, const char *, - struct cifs_sb_info *); + struct cifs_sb_info *, struct dentry *); /* open, rename and delete file */ int (*rename_pending_delete)(const char *, struct dentry *, const unsigned int); diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h index a841bf4967fa..ef98c840791f 100644 --- a/fs/smb/client/cifsproto.h +++ b/fs/smb/client/cifsproto.h @@ -402,7 +402,8 @@ extern int CIFSSMBSetFileDisposition(const unsigned int xid, __u32 pid_of_opener); extern int CIFSSMBSetEOF(const unsigned int xid, struct cifs_tcon *tcon, const char *file_name, __u64 size, - struct cifs_sb_info *cifs_sb, bool set_allocation); + struct cifs_sb_info *cifs_sb, bool set_allocation, + struct dentry *dentry); extern int CIFSSMBSetFileSize(const unsigned int xid, struct cifs_tcon *tcon, struct cifsFileInfo *cfile, __u64 size, bool set_allocation); @@ -438,7 +439,8 @@ extern int CIFSPOSIXDelFile(const unsigned int xid, struct cifs_tcon *tcon, const struct nls_table *nls_codepage, int remap_special_chars); extern int CIFSSMBDelFile(const unsigned int xid, struct cifs_tcon *tcon, - const char *name, struct cifs_sb_info *cifs_sb); + const char *name, struct cifs_sb_info *cifs_sb, + struct dentry *dentry); int CIFSSMBRename(const unsigned int xid, struct cifs_tcon *tcon, struct dentry *source_dentry, const char *from_name, const char *to_name, diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c index 01e89070df5a..301189ee1335 100644 --- a/fs/smb/client/cifssmb.c +++ b/fs/smb/client/cifssmb.c @@ -738,7 +738,7 @@ CIFSPOSIXDelFile(const unsigned int xid, struct cifs_tcon *tcon, int CIFSSMBDelFile(const unsigned int xid, struct cifs_tcon *tcon, const char *name, - struct cifs_sb_info *cifs_sb) + struct cifs_sb_info *cifs_sb, struct dentry *dentry) { DELETE_FILE_REQ *pSMB = NULL; DELETE_FILE_RSP *pSMBr = NULL; @@ -4993,7 +4993,7 @@ CIFSSMBQFSPosixInfo(const unsigned int xid, struct cifs_tcon *tcon, int CIFSSMBSetEOF(const unsigned int xid, struct cifs_tcon *tcon, const char *file_name, __u64 size, struct cifs_sb_info *cifs_sb, - bool set_allocation) + bool set_allocation, struct dentry *dentry) { struct smb_com_transaction2_spi_req *pSMB = NULL; struct smb_com_transaction2_spi_rsp *pSMBr = NULL; diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c index d02f8ba29cb5..3073eac989ea 100644 --- a/fs/smb/client/inode.c +++ b/fs/smb/client/inode.c @@ -1846,7 +1846,7 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry) goto psx_del_no_retry; } - rc = server->ops->unlink(xid, tcon, full_path, cifs_sb); + rc = server->ops->unlink(xid, tcon, full_path, cifs_sb, dentry); psx_del_no_retry: if (!rc) { @@ -2797,7 +2797,7 @@ void cifs_setsize(struct inode *inode, loff_t offset) static int cifs_set_file_size(struct inode *inode, struct iattr *attrs, - unsigned int xid, const char *full_path) + unsigned int xid, const char *full_path, struct dentry *dentry) { int rc; struct cifsFileInfo *open_file; @@ -2848,7 +2848,7 @@ cifs_set_file_size(struct inode *inode, struct iattr *attrs, */ if (server->ops->set_path_size) rc = server->ops->set_path_size(xid, tcon, full_path, - attrs->ia_size, cifs_sb, false); + attrs->ia_size, cifs_sb, false, dentry); else rc = -ENOSYS; cifs_dbg(FYI, "SetEOF by path (setattrs) rc = %d\n", rc); @@ -2938,7 +2938,7 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs) rc = 0; if (attrs->ia_valid & ATTR_SIZE) { - rc = cifs_set_file_size(inode, attrs, xid, full_path); + rc = cifs_set_file_size(inode, attrs, xid, full_path, direntry); if (rc != 0) goto out; } @@ -3105,7 +3105,7 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs) } if (attrs->ia_valid & ATTR_SIZE) { - rc = cifs_set_file_size(inode, attrs, xid, full_path); + rc = cifs_set_file_size(inode, attrs, xid, full_path, direntry); if (rc != 0) goto cifs_setattr_exit; } diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c index 05818cd6d932..69f3442c5b96 100644 --- a/fs/smb/client/smb2inode.c +++ b/fs/smb/client/smb2inode.c @@ -98,7 +98,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, __u32 desired_access, __u32 create_disposition, __u32 create_options, umode_t mode, struct kvec *in_iov, int *cmds, int num_cmds, struct cifsFileInfo *cfile, - struct kvec *out_iov, int *out_buftype) + struct kvec *out_iov, int *out_buftype, struct dentry *dentry) { struct reparse_data_buffer *rbuf; @@ -115,6 +115,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, int resp_buftype[MAX_COMPOUND]; struct smb2_query_info_rsp *qi_rsp = NULL; struct cifs_open_info_data *idata; + struct inode *inode = NULL; int flags = 0; __u8 delete_pending[8] = {1, 0, 0, 0, 0, 0, 0, 0}; unsigned int size[2]; @@ -152,6 +153,15 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, goto finished; } + /* if there is an existing lease, reuse it */ + if (dentry) { + inode = d_inode(dentry); + if (CIFS_I(inode)->lease_granted && server->ops->get_lease_key) { + oplock = SMB2_OPLOCK_LEVEL_LEASE; + server->ops->get_lease_key(inode, &fid); + } + } + vars->oparms = (struct cifs_open_parms) { .tcon = tcon, .path = full_path, @@ -747,7 +757,7 @@ int smb2_query_path_info(const unsigned int xid, rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, FILE_READ_ATTRIBUTES, FILE_OPEN, create_options, ACL_NO_MODE, in_iov, - cmds, 1, cfile, out_iov, out_buftype); + cmds, 1, cfile, out_iov, out_buftype, NULL); hdr = out_iov[0].iov_base; /* * If first iov is unset, then SMB session was dropped or we've got a @@ -779,7 +789,7 @@ int smb2_query_path_info(const unsigned int xid, rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, FILE_READ_ATTRIBUTES, FILE_OPEN, create_options, ACL_NO_MODE, in_iov, - cmds, num_cmds, cfile, NULL, NULL); + cmds, num_cmds, cfile, NULL, NULL, NULL); break; case -EREMOTE: break; @@ -811,7 +821,7 @@ smb2_mkdir(const unsigned int xid, struct inode *parent_inode, umode_t mode, FILE_WRITE_ATTRIBUTES, FILE_CREATE, CREATE_NOT_FILE, mode, NULL, &(int){SMB2_OP_MKDIR}, 1, - NULL, NULL, NULL); + NULL, NULL, NULL, NULL); } void @@ -836,7 +846,7 @@ smb2_mkdir_setinfo(struct inode *inode, const char *name, FILE_WRITE_ATTRIBUTES, FILE_CREATE, CREATE_NOT_FILE, ACL_NO_MODE, &in_iov, &(int){SMB2_OP_SET_INFO}, 1, - cfile, NULL, NULL); + cfile, NULL, NULL, NULL); if (tmprc == 0) cifs_i->cifsAttrs = dosattrs; } @@ -850,25 +860,26 @@ smb2_rmdir(const unsigned int xid, struct cifs_tcon *tcon, const char *name, DELETE, FILE_OPEN, CREATE_NOT_FILE, ACL_NO_MODE, NULL, &(int){SMB2_OP_RMDIR}, 1, - NULL, NULL, NULL); + NULL, NULL, NULL, NULL); } int smb2_unlink(const unsigned int xid, struct cifs_tcon *tcon, const char *name, - struct cifs_sb_info *cifs_sb) + struct cifs_sb_info *cifs_sb, struct dentry *dentry) { return smb2_compound_op(xid, tcon, cifs_sb, name, DELETE, FILE_OPEN, CREATE_DELETE_ON_CLOSE | OPEN_REPARSE_POINT, ACL_NO_MODE, NULL, &(int){SMB2_OP_DELETE}, 1, - NULL, NULL, NULL); + NULL, NULL, NULL, dentry); } static int smb2_set_path_attr(const unsigned int xid, struct cifs_tcon *tcon, const char *from_name, const char *to_name, struct cifs_sb_info *cifs_sb, __u32 create_options, __u32 access, - int command, struct cifsFileInfo *cfile) + int command, struct cifsFileInfo *cfile, + struct dentry *dentry) { struct kvec in_iov; __le16 *smb2_to_name = NULL; @@ -883,7 +894,7 @@ static int smb2_set_path_attr(const unsigned int xid, struct cifs_tcon *tcon, in_iov.iov_len = 2 * UniStrnlen((wchar_t *)smb2_to_name, PATH_MAX); rc = smb2_compound_op(xid, tcon, cifs_sb, from_name, access, FILE_OPEN, create_options, ACL_NO_MODE, - &in_iov, &command, 1, cfile, NULL, NULL); + &in_iov, &command, 1, cfile, NULL, NULL, dentry); smb2_rename_path: kfree(smb2_to_name); return rc; @@ -902,7 +913,7 @@ int smb2_rename_path(const unsigned int xid, cifs_get_writable_path(tcon, from_name, FIND_WR_WITH_DELETE, &cfile); return smb2_set_path_attr(xid, tcon, from_name, to_name, cifs_sb, - co, DELETE, SMB2_OP_RENAME, cfile); + co, DELETE, SMB2_OP_RENAME, cfile, source_dentry); } int smb2_create_hardlink(const unsigned int xid, @@ -915,13 +926,14 @@ int smb2_create_hardlink(const unsigned int xid, return smb2_set_path_attr(xid, tcon, from_name, to_name, cifs_sb, co, FILE_READ_ATTRIBUTES, - SMB2_OP_HARDLINK, NULL); + SMB2_OP_HARDLINK, NULL, NULL); } int smb2_set_path_size(const unsigned int xid, struct cifs_tcon *tcon, const char *full_path, __u64 size, - struct cifs_sb_info *cifs_sb, bool set_alloc) + struct cifs_sb_info *cifs_sb, bool set_alloc, + struct dentry *dentry) { struct cifsFileInfo *cfile; struct kvec in_iov; @@ -934,7 +946,7 @@ smb2_set_path_size(const unsigned int xid, struct cifs_tcon *tcon, FILE_WRITE_DATA, FILE_OPEN, 0, ACL_NO_MODE, &in_iov, &(int){SMB2_OP_SET_EOF}, 1, - cfile, NULL, NULL); + cfile, NULL, NULL, dentry); } int @@ -963,7 +975,7 @@ smb2_set_file_info(struct inode *inode, const char *full_path, FILE_WRITE_ATTRIBUTES, FILE_OPEN, 0, ACL_NO_MODE, &in_iov, &(int){SMB2_OP_SET_INFO}, 1, - cfile, NULL, NULL); + cfile, NULL, NULL, NULL); cifs_put_tlink(tlink); return rc; } @@ -998,7 +1010,7 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data, cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile); rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, da, cd, co, ACL_NO_MODE, in_iov, - cmds, 2, cfile, NULL, NULL); + cmds, 2, cfile, NULL, NULL, NULL); if (!rc) { rc = smb311_posix_get_inode_info(&new, full_path, data, sb, xid); @@ -1008,7 +1020,7 @@ struct inode *smb2_get_reparse_inode(struct cifs_open_info_data *data, cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile); rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, da, cd, co, ACL_NO_MODE, in_iov, - cmds, 2, cfile, NULL, NULL); + cmds, 2, cfile, NULL, NULL, NULL); if (!rc) { rc = cifs_get_inode_info(&new, full_path, data, sb, xid, NULL); @@ -1036,7 +1048,7 @@ int smb2_query_reparse_point(const unsigned int xid, FILE_READ_ATTRIBUTES, FILE_OPEN, OPEN_REPARSE_POINT, ACL_NO_MODE, &in_iov, &(int){SMB2_OP_GET_REPARSE}, 1, - cfile, NULL, NULL); + cfile, NULL, NULL, NULL); if (rc) goto out; diff --git a/fs/smb/client/smb2proto.h b/fs/smb/client/smb2proto.h index b3069911e9dd..221143788a1c 100644 --- a/fs/smb/client/smb2proto.h +++ b/fs/smb/client/smb2proto.h @@ -75,7 +75,8 @@ int smb2_query_path_info(const unsigned int xid, struct cifs_open_info_data *data); extern int smb2_set_path_size(const unsigned int xid, struct cifs_tcon *tcon, const char *full_path, __u64 size, - struct cifs_sb_info *cifs_sb, bool set_alloc); + struct cifs_sb_info *cifs_sb, bool set_alloc, + struct dentry *dentry); extern int smb2_set_file_info(struct inode *inode, const char *full_path, FILE_BASIC_INFO *buf, const unsigned int xid); extern int smb311_posix_mkdir(const unsigned int xid, struct inode *inode, @@ -91,7 +92,8 @@ extern void smb2_mkdir_setinfo(struct inode *inode, const char *full_path, extern int smb2_rmdir(const unsigned int xid, struct cifs_tcon *tcon, const char *name, struct cifs_sb_info *cifs_sb); extern int smb2_unlink(const unsigned int xid, struct cifs_tcon *tcon, - const char *name, struct cifs_sb_info *cifs_sb); + const char *name, struct cifs_sb_info *cifs_sb, + struct dentry *dentry); int smb2_rename_path(const unsigned int xid, struct cifs_tcon *tcon, struct dentry *source_dentry, From patchwork Wed Mar 6 03:43:52 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Meetakshi Setiya X-Patchwork-Id: 13583318 Received: from mail-oo1-f42.google.com (mail-oo1-f42.google.com [209.85.161.42]) (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 09DB814AB2; Wed, 6 Mar 2024 03:44:02 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.161.42 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709696644; cv=none; b=K7CKBxFKrCBUhp+IFVgMdQZVv5DGD83XZL24EfRIa3nU85YPGYO7s3SBNZn9pBYoSBvEmBN7biIgKselCr3mdQ8a/P9hDl7WjOTTqx79h9PQc2uLDX67QMqaYu8cqHXcLf/C8QCLTLI2Rhrjibp6Je3Ofsw+3xcEomZV65lGIhg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709696644; c=relaxed/simple; bh=r6rn3rDdf+TNYJsnbW86PV4rErqk5qquWoWPIqhEmbI=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=GWrVO1kVkCalZlpotYKZvJnAx6kLQiXZjOeq6QdNX/xMtDE/Jm4spFv8DL3TT9spJ/2TKcaET5ITL+sSmqqvcV3LmVsJMM4uqDtZ4pcH3+4vbSpVqjazWD+ZoLHUq4XhHUiiKKEb09hshdzyU4/fgrniIXI4EDXEBk9TJWjUZbU= 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=chqqPXER; arc=none smtp.client-ip=209.85.161.42 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="chqqPXER" Received: by mail-oo1-f42.google.com with SMTP id 006d021491bc7-59fb0b5b47eso219951eaf.3; Tue, 05 Mar 2024 19:44:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1709696642; x=1710301442; 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=+IrVmSfa1fxZtNJO3Co2iIfu8tPAcVFJOCNWkXReefY=; b=chqqPXERKH6C+55BUGx4N5UILLRxqkoEYKo69Kd3Gs5JXatdgwOkXfPuYtMd78Vkbs Url0JEDYHz893bTSgmW1fydc+XcLdsRgtmUM8QkSvLt3j6U81iVl/VpGyE8UgKrqTmLt 2ypEcDr7rLoMr/bKdGL0Kpyn1lCgOoGmE3l85FZFJxXO3Z2IzZMqyr93G7BIKDOMqjbt s8xos3cyroZuy1zKsrXe+V1tXT0m8ZNdXQx3cc8B+ntMPeQPwbKpyawcEX5+QQIeAKt0 Z/tPoNkDWRPOTre8CCI4/qldAfgEKWZbq/29qjlY7vHHs7SsMYp1m3rRED7itw+IgCld VxrA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709696642; x=1710301442; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=+IrVmSfa1fxZtNJO3Co2iIfu8tPAcVFJOCNWkXReefY=; b=opmt7dXJCX5dib4EoMSf4bIN5WemgT4IA6yOdVz2dKeAH/Sy6mdRbMp7hdr1JU9hy7 8P8rtAGfCgGSe+EV1JljJRZbXGp0vSrqqDFui3OQFvRwSuCZ//JORu6qYQfz5PIfUoc+ g3vTyIfkhQgptaJWiJTeC50M5Tjc/6npYypkA/rVWOPNCtMPdOBjVITBlFeBhHm1638Z PyqTl9/UUgBPxylGtATqfBX8ENj4jZPbol/55jqdsf/9lgOQPUyguN3s3dJCsDM1THTQ mDdVwjR17ZebAo0Acjjyuw2rOQyc4C9jlnD/bG9jZKDeIlgFCLNU36UwcP1TrGVzpvm2 hl5w== X-Forwarded-Encrypted: i=1; AJvYcCUVZq7aVaIynoqFdc8EvmKKjxsfYQrZ3cro29Cmv3pzctFytUfoD5ZI1HM13nng6VnfOYpT0vyb+/RT93rTIw/O7C2pI/GJlEu31PDwQVkS1zuvszQKVZzcHVvCAjr8wdmUS1F/jwmY8A== X-Gm-Message-State: AOJu0YzLc2afGjp7ikvSL3uDGRrLDkHB8DGKUncrvyElGEsBvm0CKxVf tg/zfYELKwwyVDG57cByKL29QBwgEFIrZLQTpYfZo0pv56kykT4+ X-Google-Smtp-Source: AGHT+IGWW2iuKnXDx0P2RTTWYXUhklOegA9YzBx297q4hdTh3tsOV2TC3PdhcHjqXm0JXCw3CeI4Mg== X-Received: by 2002:a05:6358:6f8a:b0:17c:1db2:4ba4 with SMTP id s10-20020a0563586f8a00b0017c1db24ba4mr4533736rwn.4.1709696641953; Tue, 05 Mar 2024 19:44:01 -0800 (PST) Received: from met-Virtual-Machine.. ([131.107.159.31]) by smtp.gmail.com with ESMTPSA id j5-20020a654305000000b005db034d1514sm8639808pgq.82.2024.03.05.19.44.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 Mar 2024 19:44:01 -0800 (PST) From: meetakshisetiyaoss@gmail.com To: sfrench@samba.org, pc@manguebit.com, ronniesahlberg@gmail.com, sprasad@microsoft.com, nspmangalore@gmail.com, tom@talpey.com, linux-cifs@vger.kernel.org, linux-kernel@vger.kernel.org, samba-technical@lists.samba.org, bharathsm.hsk@gmail.com Cc: Meetakshi Setiya Subject: [PATCH 2/3] smb: client: do not defer close open handles to deleted files Date: Tue, 5 Mar 2024 22:43:52 -0500 Message-Id: <20240306034353.190039-2-meetakshisetiyaoss@gmail.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240306034353.190039-1-meetakshisetiyaoss@gmail.com> References: <20240306034353.190039-1-meetakshisetiyaoss@gmail.com> Precedence: bulk X-Mailing-List: linux-cifs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Meetakshi Setiya When a file/dentry has been deleted before closing all its open handles, currently, closing them can add them to the deferred close list. This can lead to problems in creating file with the same name when the file is re-created before the deferred close completes. This issue was seen while reusing a client's already existing lease on a file for compound operations and xfstest 591 failed because of the deferred close handle that remained valid even after the file was deleted and was being reused to create a file with the same name. The server in this case returns an error on open with STATUS_DELETE_PENDING. Recreating the file would fail till the deferred handles are closed (duration specified in closetimeo). This patch fixes the issue by flagging all open handles for the deleted file (file path to be precise) by setting status_file_deleted to true in the cifsFileInfo structure. As per the information classes specified in MS-FSCC, SMB2 query info response from the server has a DeletePending field, set to true to indicate that deletion has been requested on that file. If this is the case, flag the open handles for this file too. When doing close in cifs_close for each of these handles, check the value of this boolean field and do not defer close these handles if the corresponding filepath has been deleted. Signed-off-by: Meetakshi Setiya --- fs/smb/client/cifsglob.h | 1 + fs/smb/client/cifsproto.h | 4 ++++ fs/smb/client/file.c | 3 ++- fs/smb/client/inode.c | 28 +++++++++++++++++++++++++--- fs/smb/client/misc.c | 34 ++++++++++++++++++++++++++++++++++ fs/smb/client/smb2inode.c | 9 ++++++++- 6 files changed, 74 insertions(+), 5 deletions(-) diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h index 50f7e47c2229..a88c8328b29c 100644 --- a/fs/smb/client/cifsglob.h +++ b/fs/smb/client/cifsglob.h @@ -1417,6 +1417,7 @@ struct cifsFileInfo { bool invalidHandle:1; /* file closed via session abend */ bool swapfile:1; bool oplock_break_cancelled:1; + bool status_file_deleted:1; /* file has been deleted */ unsigned int oplock_epoch; /* epoch from the lease break */ __u32 oplock_level; /* oplock/lease level from the lease break */ int count; diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h index ef98c840791f..1f46e0db6e92 100644 --- a/fs/smb/client/cifsproto.h +++ b/fs/smb/client/cifsproto.h @@ -296,6 +296,10 @@ extern void cifs_close_all_deferred_files(struct cifs_tcon *cifs_tcon); extern void cifs_close_deferred_file_under_dentry(struct cifs_tcon *cifs_tcon, const char *path); + +extern void cifs_mark_open_handles_for_deleted_file(struct inode *inode, + const char *path); + extern struct TCP_Server_Info * cifs_get_tcp_session(struct smb3_fs_context *ctx, struct TCP_Server_Info *primary_server); diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c index b75282c204da..46951f403d31 100644 --- a/fs/smb/client/file.c +++ b/fs/smb/client/file.c @@ -483,6 +483,7 @@ struct cifsFileInfo *cifs_new_fileinfo(struct cifs_fid *fid, struct file *file, cfile->uid = current_fsuid(); cfile->dentry = dget(dentry); cfile->f_flags = file->f_flags; + cfile->status_file_deleted = false; cfile->invalidHandle = false; cfile->deferred_close_scheduled = false; cfile->tlink = cifs_get_tlink(tlink); @@ -1085,7 +1086,7 @@ int cifs_close(struct inode *inode, struct file *file) if ((cifs_sb->ctx->closetimeo && cinode->oplock == CIFS_CACHE_RHW_FLG) && cinode->lease_granted && !test_bit(CIFS_INO_CLOSE_ON_LOCK, &cinode->flags) && - dclose) { + dclose && !(cfile->status_file_deleted)) { if (test_and_clear_bit(CIFS_INO_MODIFIED_ATTR, &cinode->flags)) { inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode)); diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c index 3073eac989ea..3242e3b74386 100644 --- a/fs/smb/client/inode.c +++ b/fs/smb/client/inode.c @@ -893,6 +893,9 @@ cifs_get_file_info(struct file *filp) struct cifsFileInfo *cfile = filp->private_data; struct cifs_tcon *tcon = tlink_tcon(cfile->tlink); struct TCP_Server_Info *server = tcon->ses->server; + struct dentry *dentry = filp->f_path.dentry; + void *page = alloc_dentry_path(); + const unsigned char *path; if (!server->ops->query_file_info) return -ENOSYS; @@ -907,7 +910,14 @@ cifs_get_file_info(struct file *filp) data.symlink = true; data.reparse.tag = IO_REPARSE_TAG_SYMLINK; } + path = build_path_from_dentry(dentry, page); + if (IS_ERR(path)) { + free_dentry_path(page); + return PTR_ERR(path); + } cifs_open_info_to_fattr(&fattr, &data, inode->i_sb); + if (fattr.cf_flags & CIFS_FATTR_DELETE_PENDING) + cifs_mark_open_handles_for_deleted_file(inode, path); break; case -EREMOTE: cifs_create_junction_fattr(&fattr, inode->i_sb); @@ -937,6 +947,7 @@ cifs_get_file_info(struct file *filp) rc = cifs_fattr_to_inode(inode, &fattr); cgfi_exit: cifs_free_open_info(&data); + free_dentry_path(page); free_xid(xid); return rc; } @@ -1075,6 +1086,7 @@ static int reparse_info_to_fattr(struct cifs_open_info_data *data, struct kvec rsp_iov, *iov = NULL; int rsp_buftype = CIFS_NO_BUFFER; u32 tag = data->reparse.tag; + struct inode *inode = NULL; int rc = 0; if (!tag && server->ops->query_reparse_point) { @@ -1114,8 +1126,12 @@ static int reparse_info_to_fattr(struct cifs_open_info_data *data, if (tcon->posix_extensions) smb311_posix_info_to_fattr(fattr, data, sb); - else + else { cifs_open_info_to_fattr(fattr, data, sb); + inode = cifs_iget(sb, fattr); + if (inode && fattr->cf_flags & CIFS_FATTR_DELETE_PENDING) + cifs_mark_open_handles_for_deleted_file(inode, full_path); + } out: fattr->cf_cifstag = data->reparse.tag; free_rsp_buf(rsp_buftype, rsp_iov.iov_base); @@ -1170,6 +1186,8 @@ static int cifs_get_fattr(struct cifs_open_info_data *data, full_path, fattr); } else { cifs_open_info_to_fattr(fattr, data, sb); + if (fattr->cf_flags & CIFS_FATTR_DELETE_PENDING) + cifs_mark_open_handles_for_deleted_file(*inode, full_path); } break; case -EREMOTE: @@ -1850,16 +1868,20 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry) psx_del_no_retry: if (!rc) { - if (inode) + if (inode) { + cifs_mark_open_handles_for_deleted_file(inode, full_path); cifs_drop_nlink(inode); + } } else if (rc == -ENOENT) { d_drop(dentry); } else if (rc == -EBUSY) { if (server->ops->rename_pending_delete) { rc = server->ops->rename_pending_delete(full_path, dentry, xid); - if (rc == 0) + if (rc == 0) { + cifs_mark_open_handles_for_deleted_file(inode, full_path); cifs_drop_nlink(inode); + } } } else if ((rc == -EACCES) && (dosattr == 0) && inode) { attrs = kzalloc(sizeof(*attrs), GFP_KERNEL); diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c index 0748d7b757b9..9428a0db7718 100644 --- a/fs/smb/client/misc.c +++ b/fs/smb/client/misc.c @@ -853,6 +853,40 @@ cifs_close_deferred_file_under_dentry(struct cifs_tcon *tcon, const char *path) free_dentry_path(page); } +/* + * If a dentry has been deleted, all corresponding open handles should know that + * so that we do not defer close them. + */ +void cifs_mark_open_handles_for_deleted_file(struct inode *inode, + const char *path) +{ + struct cifsFileInfo *cfile; + void *page; + const char *full_path; + struct cifsInodeInfo *cinode = CIFS_I(inode); + + page = alloc_dentry_path(); + spin_lock(&cinode->open_file_lock); + + /* + * note: we need to construct path from dentry and compare only if the + * inode has any hardlinks. When number of hardlinks is 1, we can just + * mark all open handles since they are going to be from the same file. + */ + if (inode->i_nlink > 1) { + list_for_each_entry(cfile, &cinode->openFileList, flist) { + full_path = build_path_from_dentry(cfile->dentry, page); + if (!IS_ERR(full_path) && strcmp(full_path, path) == 0) + cfile->status_file_deleted = true; + } + } else { + list_for_each_entry(cfile, &cinode->openFileList, flist) + cfile->status_file_deleted = true; + } + spin_unlock(&cinode->open_file_lock); + free_dentry_path(page); +} + /* parses DFS referral V3 structure * caller is responsible for freeing target_nodes * returns: diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c index 69f3442c5b96..429d83d31280 100644 --- a/fs/smb/client/smb2inode.c +++ b/fs/smb/client/smb2inode.c @@ -561,8 +561,15 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, case SMB2_OP_DELETE: if (rc) trace_smb3_delete_err(xid, ses->Suid, tcon->tid, rc); - else + else { + /* + * If dentry (hence, inode) is NULL, lease break is going to + * take care of degrading leases on handles for deleted files. + */ + if (inode) + cifs_mark_open_handles_for_deleted_file(inode, full_path); trace_smb3_delete_done(xid, ses->Suid, tcon->tid); + } break; case SMB2_OP_MKDIR: if (rc) From patchwork Wed Mar 6 03:43:53 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Meetakshi Setiya X-Patchwork-Id: 13583319 Received: from mail-pj1-f46.google.com (mail-pj1-f46.google.com [209.85.216.46]) (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 6F325179B2; Wed, 6 Mar 2024 03:44:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.46 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709696648; cv=none; b=BbB1qztJUg8WY3sQ1QBFV7yBZtg77s4LObX7v1W46BCqSKIP2YRTxI+fQ6uuzAad6x6StlLSNgcAlXTZle/YMnu2gMrDNy13sr5dnoJEkEltc9KDUBMydvvarsNSDmydOFgalxvKhxP/NMpQvKH63w9viX/hWJqyPXRGib/QPes= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709696648; c=relaxed/simple; bh=C/jtdEDa4Us6RqtlJJcTg3kI3YdurrJ2pc7QD8o8W9o=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=j8bTQaZdSoPj9INAcdPdr3La56m/Ym6fUYuPswbj6GAzbpnd0PVw2hWbdmYXrAHp1luTJxmJa1Au/FrxTGaCTGqKy8lhl+bXDB9cZooBip6VEFlKYv6iBPGUyMpuqYt/2+pe7/+4w9nNP2pcCUT9aY25PvuQjG7DNHc0hgXkutM= 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=jjeWUv6B; arc=none smtp.client-ip=209.85.216.46 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="jjeWUv6B" Received: by mail-pj1-f46.google.com with SMTP id 98e67ed59e1d1-29aa91e173dso1033777a91.0; Tue, 05 Mar 2024 19:44:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1709696646; x=1710301446; 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=nNFqoOs60d2BJyKt3WPYYQhsfs7ZhiZxWwuBcouXhmU=; b=jjeWUv6B7omHsDIKboK7IdS4Ep96dOqrsDEzV22Rw51efBB8Tu2kvLFuE+Ix58OuAZ 9Uwo+H9C+X//hqLHWP+dbXVQkw6pfTFf7jth+FLDITUES6KXqK8EYApus/NzXRlKS/uF bKvNqpQW1DM1+VhfJlSV+obpNRBuyKURXy69nvQBZRZyTLfZaJ1/YaSgtwO1Oo6Vk5Jb m9qfr83kI9JCm/2n2S/cOy8ydkcL7+d0DQX7V670NFe+7wdP6r9hBvH+aB5tYFxaF+uO nRKbSUiybOIfcaEn+zOCuwKsWt0mm+xivHmrhA3xOD2J0uIJ7b4D4kBTTYjm9Yai0NoF vZGA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709696646; x=1710301446; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=nNFqoOs60d2BJyKt3WPYYQhsfs7ZhiZxWwuBcouXhmU=; b=dwBsW80i2hxBrB5o+GRzt//RBEZ1DqbY8SXfcMv3lErZmhHzbhWFid+icw0XtOfwU8 5g8JH4Y11FA/ZyIjEbQfRUYVWZEONygRZSP/xCe/soq6RT++LMOQ69CxwFoGQDKhNsKn g1SyX9/Lmf7q6fDaex31k6ZQfrJPIguHBJA4g92x+L92psAm+fh+0obcMEEkf/aBO1qq xLwt6uhmaEFlKi6bJDtP8hagIdp+6BSiB//bo7jKb3dUQBPHPibj2lMrmW1I2aEyg9qo R/AvuofjtB7ubQ4Dw6bw/IfoqIMYu7/YZ/wL1OO71bG9JB4FAXA+tdcrj0ZjHGsfBemx EOig== X-Forwarded-Encrypted: i=1; AJvYcCV+y481NwfsaXT8/2OZgox8RxKVDzoD/+B1tpyOOKd15tdEy/l/HbsBIrSkieLf4VjN67nPYRbinp9EdD0bqwEH38Om3u4kwx/VrQrnHRDWl99q0xlTdZIa25kEGmQl0qKp9zKNyP4DVg== X-Gm-Message-State: AOJu0YxKx5r9Ocj6TNxE9zr+/7DVmL8PDZSMew8+QAaJRiZ0sfZK2I9G IqjBb0le6E2R9kogEwkWKxQrkezt/opYk9+tzgrLDU/kYZzD+2cb X-Google-Smtp-Source: AGHT+IEnShgC5axrveTeLqP6fomzKQezN5CS8/lEuoMfRyWYwejQ0SqE1+OXr9WLcHVrkgVqP9SlgQ== X-Received: by 2002:a17:90a:c7c4:b0:29a:6c13:f0a with SMTP id gf4-20020a17090ac7c400b0029a6c130f0amr11038527pjb.16.1709696645660; Tue, 05 Mar 2024 19:44:05 -0800 (PST) Received: from met-Virtual-Machine.. ([131.107.159.31]) by smtp.gmail.com with ESMTPSA id j5-20020a654305000000b005db034d1514sm8639808pgq.82.2024.03.05.19.44.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 Mar 2024 19:44:05 -0800 (PST) From: meetakshisetiyaoss@gmail.com To: sfrench@samba.org, pc@manguebit.com, ronniesahlberg@gmail.com, sprasad@microsoft.com, nspmangalore@gmail.com, tom@talpey.com, linux-cifs@vger.kernel.org, linux-kernel@vger.kernel.org, samba-technical@lists.samba.org, bharathsm.hsk@gmail.com Cc: Meetakshi Setiya Subject: [PATCH 3/3] smb: client: retry compound request without reusing lease Date: Tue, 5 Mar 2024 22:43:53 -0500 Message-Id: <20240306034353.190039-3-meetakshisetiyaoss@gmail.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240306034353.190039-1-meetakshisetiyaoss@gmail.com> References: <20240306034353.190039-1-meetakshisetiyaoss@gmail.com> Precedence: bulk X-Mailing-List: linux-cifs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Meetakshi Setiya There is a shortcoming in the current implementation of the file lease mechanism exposed when the lease keys were attempted to be reused for unlink, rename and set_path_size operations for a client. As per MS-SMB2, lease keys are associated with the file name. Linux smb client maintains lease keys with the inode. If the file has any hardlinks, it is possible that the lease for a file be wrongly reused for an operation on the hardlink or vice versa. In these cases, the mentioned compound operations fail with STATUS_INVALID_PARAMETER. This patch adds a fallback to the old mechanism of not sending any lease with these compound operations if the request with lease key fails with STATUS_INVALID_PARAMETER. Resending the same request without lease key should not hurt any functionality, but might impact performance especially in cases where the error is not because of the usage of wrong lease key and we might end up doing an extra roundtrip. Signed-off-by: Meetakshi Setiya --- fs/smb/client/smb2inode.c | 41 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c index 429d83d31280..f697c14cd8c6 100644 --- a/fs/smb/client/smb2inode.c +++ b/fs/smb/client/smb2inode.c @@ -154,6 +154,17 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, } /* if there is an existing lease, reuse it */ + + /* + * note: files with hardlinks cause unexpected behaviour. As per MS-SMB2, + * lease keys are associated with the filepath. We are maintaining lease keys + * with the inode on the client. If the file has hardlinks, it is possible + * that the lease for a file be reused for an operation on its hardlink or + * vice versa. + * As a workaround, send request using an existing lease key and if the server + * returns STATUS_INVALID_PARAMETER, which maps to EINVAL, send the request + * again without the lease. + */ if (dentry) { inode = d_inode(dentry); if (CIFS_I(inode)->lease_granted && server->ops->get_lease_key) { @@ -874,11 +885,20 @@ int smb2_unlink(const unsigned int xid, struct cifs_tcon *tcon, const char *name, struct cifs_sb_info *cifs_sb, struct dentry *dentry) { - return smb2_compound_op(xid, tcon, cifs_sb, name, DELETE, FILE_OPEN, + int rc = smb2_compound_op(xid, tcon, cifs_sb, name, DELETE, FILE_OPEN, CREATE_DELETE_ON_CLOSE | OPEN_REPARSE_POINT, ACL_NO_MODE, NULL, &(int){SMB2_OP_DELETE}, 1, NULL, NULL, NULL, dentry); + if (rc == -EINVAL) { + cifs_dbg(FYI, "invalid lease key, resending request without lease"); + rc = smb2_compound_op(xid, tcon, cifs_sb, name, DELETE, FILE_OPEN, + CREATE_DELETE_ON_CLOSE | OPEN_REPARSE_POINT, + ACL_NO_MODE, NULL, + &(int){SMB2_OP_DELETE}, 1, + NULL, NULL, NULL, NULL); + } + return rc; } static int smb2_set_path_attr(const unsigned int xid, struct cifs_tcon *tcon, @@ -919,8 +939,14 @@ int smb2_rename_path(const unsigned int xid, drop_cached_dir_by_name(xid, tcon, from_name, cifs_sb); cifs_get_writable_path(tcon, from_name, FIND_WR_WITH_DELETE, &cfile); - return smb2_set_path_attr(xid, tcon, from_name, to_name, cifs_sb, + int rc = smb2_set_path_attr(xid, tcon, from_name, to_name, cifs_sb, co, DELETE, SMB2_OP_RENAME, cfile, source_dentry); + if (rc == -EINVAL) { + cifs_dbg(FYI, "invalid lease key, resending request without lease"); + rc = smb2_set_path_attr(xid, tcon, from_name, to_name, cifs_sb, + co, DELETE, SMB2_OP_RENAME, cfile, NULL); + } + return rc; } int smb2_create_hardlink(const unsigned int xid, @@ -949,11 +975,20 @@ smb2_set_path_size(const unsigned int xid, struct cifs_tcon *tcon, in_iov.iov_base = &eof; in_iov.iov_len = sizeof(eof); cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile); - return smb2_compound_op(xid, tcon, cifs_sb, full_path, + int rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, FILE_WRITE_DATA, FILE_OPEN, 0, ACL_NO_MODE, &in_iov, &(int){SMB2_OP_SET_EOF}, 1, cfile, NULL, NULL, dentry); + if (rc == -EINVAL) { + cifs_dbg(FYI, "invalid lease key, resending request without lease"); + rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, + FILE_WRITE_DATA, FILE_OPEN, + 0, ACL_NO_MODE, &in_iov, + &(int){SMB2_OP_SET_EOF}, 1, + cfile, NULL, NULL, NULL); + } + return rc; } int