From: Steve French <smfrench@gmail.com>
To: Paulo Alcantara <pc@manguebit.org>
Cc: David Howells <dhowells@redhat.com>,
Frank Sorenson <sorenson@redhat.com>,
linux-cifs@vger.kernel.org
Subject: Re: [PATCH] smb: client: handle unlink(2) of files open by different clients
Date: Sat, 20 Sep 2025 12:02:52 -0500 [thread overview]
Message-ID: <CAH2r5mvEUjSu=tRUSg_fb=FV1PsuZPuViFFpXGNHFFZ2s8GaBQ@mail.gmail.com> (raw)
In-Reply-To: <20250919171315.1137337-1-pc@manguebit.org>
applied to cifs-2.6.git for-next pending additional testing
On Fri, Sep 19, 2025 at 12:13 PM Paulo Alcantara <pc@manguebit.org> wrote:
>
> In order to identify whether a certain file is open by a different
> client, start the unlink process by sending a compound request of
> CREATE(DELETE_ON_CLOSE) + CLOSE with only FILE_SHARE_DELETE bit set in
> smb2_create_req::ShareAccess. If the file is currently open, then the
> server will fail the request with STATUS_SHARING_VIOLATION, in which
> case we'll map it to -EBUSY, so __cifs_unlink() will fall back to
> silly-rename the file.
>
> This fixes the following case where open(O_CREAT) fails with
> -ENOENT (STATUS_DELETE_PENDING) due to file still open by a different
> client.
>
> * Before patch
>
> $ mount.cifs //srv/share /mnt/1 -o ...,nosharesock
> $ mount.cifs //srv/share /mnt/2 -o ...,nosharesock
> $ cd /mnt/1
> $ touch foo
> $ exec 3<>foo
> $ cd /mnt/2
> $ rm foo
> $ touch foo
> touch: cannot touch 'foo': No such file or directory
> $ exec 3>&-
>
> * After patch
>
> $ mount.cifs //srv/share /mnt/1 -o ...,nosharesock
> $ mount.cifs //srv/share /mnt/2 -o ...,nosharesock
> $ cd /mnt/1
> $ touch foo
> $ exec 3<>foo
> $ cd /mnt/2
> $ rm foo
> $ touch foo
> $ exec 3>&-
>
> Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
> Reviewed-by: David Howells <dhowells@redhat.com>
> Cc: Frank Sorenson <sorenson@redhat.com>
> Cc: David Howells <dhowells@redhat.com>
> Cc: linux-cifs@vger.kernel.org
> ---
> fs/smb/client/smb2inode.c | 99 ++++++++++++++++++++++++++++++++++-----
> 1 file changed, 88 insertions(+), 11 deletions(-)
>
> diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
> index 7cadc8ca4f55..e32a3f338793 100644
> --- a/fs/smb/client/smb2inode.c
> +++ b/fs/smb/client/smb2inode.c
> @@ -1175,23 +1175,92 @@ int
> smb2_unlink(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
> struct cifs_sb_info *cifs_sb, struct dentry *dentry)
> {
> + struct kvec open_iov[SMB2_CREATE_IOV_SIZE];
> + __le16 *utf16_path __free(kfree) = NULL;
> + int retries = 0, cur_sleep = 1;
> + struct TCP_Server_Info *server;
> struct cifs_open_parms oparms;
> + struct smb2_create_req *creq;
> struct inode *inode = NULL;
> + struct smb_rqst rqst[2];
> + struct kvec rsp_iov[2];
> + struct kvec close_iov;
> + int resp_buftype[2];
> + struct cifs_fid fid;
> + int flags = 0;
> + __u8 oplock;
> int rc;
>
> - if (dentry)
> + utf16_path = cifs_convert_path_to_utf16(name, cifs_sb);
> + if (!utf16_path)
> + return -ENOMEM;
> +
> + if (smb3_encryption_required(tcon))
> + flags |= CIFS_TRANSFORM_REQ;
> +again:
> + oplock = SMB2_OPLOCK_LEVEL_NONE;
> + server = cifs_pick_channel(tcon->ses);
> +
> + memset(rqst, 0, sizeof(rqst));
> + memset(resp_buftype, 0, sizeof(resp_buftype));
> + memset(rsp_iov, 0, sizeof(rsp_iov));
> +
> + rqst[0].rq_iov = open_iov;
> + rqst[0].rq_nvec = ARRAY_SIZE(open_iov);
> +
> + oparms = CIFS_OPARMS(cifs_sb, tcon, name, DELETE | FILE_READ_ATTRIBUTES,
> + FILE_OPEN, CREATE_DELETE_ON_CLOSE |
> + OPEN_REPARSE_POINT, ACL_NO_MODE);
> + oparms.fid = &fid;
> +
> + 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);
> + }
> + }
>
> - oparms = CIFS_OPARMS(cifs_sb, tcon, name, DELETE,
> - FILE_OPEN, OPEN_REPARSE_POINT, ACL_NO_MODE);
> - rc = smb2_compound_op(xid, tcon, cifs_sb, name, &oparms,
> - NULL, &(int){SMB2_OP_UNLINK},
> - 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, &oparms,
> - NULL, &(int){SMB2_OP_UNLINK},
> - 1, NULL, NULL, NULL, NULL);
> + rc = SMB2_open_init(tcon, server,
> + &rqst[0], &oplock, &oparms, utf16_path);
> + if (rc)
> + goto err_free;
> + smb2_set_next_command(tcon, &rqst[0]);
> + creq = rqst[0].rq_iov[0].iov_base;
> + creq->ShareAccess = FILE_SHARE_DELETE_LE;
> +
> + rqst[1].rq_iov = &close_iov;
> + rqst[1].rq_nvec = 1;
> +
> + rc = SMB2_close_init(tcon, server, &rqst[1],
> + COMPOUND_FID, COMPOUND_FID, false);
> + smb2_set_related(&rqst[1]);
> + if (rc)
> + goto err_free;
> +
> + if (retries) {
> + for (int i = 0; i < ARRAY_SIZE(rqst); i++)
> + smb2_set_replay(server, &rqst[i]);
> + }
> +
> + rc = compound_send_recv(xid, tcon->ses, server, flags,
> + ARRAY_SIZE(rqst), rqst,
> + resp_buftype, rsp_iov);
> + SMB2_open_free(&rqst[0]);
> + SMB2_close_free(&rqst[1]);
> + free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base);
> + free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base);
> +
> + if (is_replayable_error(rc) &&
> + smb2_should_replay(tcon, &retries, &cur_sleep))
> + goto again;
> +
> + /* Retry compound request without lease */
> + if (rc == -EINVAL && dentry) {
> + dentry = NULL;
> + retries = 0;
> + cur_sleep = 1;
> + goto again;
> }
> /*
> * If dentry (hence, inode) is NULL, lease break is going to
> @@ -1199,6 +1268,14 @@ smb2_unlink(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
> */
> if (!rc && inode)
> cifs_mark_open_handles_for_deleted_file(inode, name);
> +
> + return rc;
> +
> +err_free:
> + SMB2_open_free(&rqst[0]);
> + SMB2_close_free(&rqst[1]);
> + free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base);
> + free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base);
> return rc;
> }
>
> --
> 2.51.0
>
--
Thanks,
Steve
prev parent reply other threads:[~2025-09-20 17:03 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-19 17:13 [PATCH] smb: client: handle unlink(2) of files open by different clients Paulo Alcantara
2025-09-20 13:58 ` Pali Rohár
2025-09-20 17:02 ` Steve French [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAH2r5mvEUjSu=tRUSg_fb=FV1PsuZPuViFFpXGNHFFZ2s8GaBQ@mail.gmail.com' \
--to=smfrench@gmail.com \
--cc=dhowells@redhat.com \
--cc=linux-cifs@vger.kernel.org \
--cc=pc@manguebit.org \
--cc=sorenson@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).