Skip to content

Commit 617fdf8

Browse files
committed
smb: client: fix data loss due to broken rename(2)
jira LE-4603 Rebuild_History Non-Buildable kernel-5.14.0-570.58.1.el9_6 commit-author Paulo Alcantara <pc@manguebit.org> commit c5ea306 Rename of open files in SMB2+ has been broken for a very long time, resulting in data loss as the CIFS client would fail the rename(2) call with -ENOENT and then removing the target file. Fix this by implementing ->rename_pending_delete() for SMB2+, which will rename busy files to random filenames (e.g. silly rename) during unlink(2) or rename(2), and then marking them to delete-on-close. Besides, introduce a FIND_WR_NO_PENDING_DELETE flag to prevent open(2) from reusing open handles that had been marked as delete pending. Handle it in cifs_get_readable_path() as well. Reported-by: Jean-Baptiste Denis <jbdenis@pasteur.fr> Closes: https://marc.info/?i=16aeb380-30d4-4551-9134-4e7d1dc833c0@pasteur.fr Reviewed-by: David Howells <dhowells@redhat.com> Cc: stable@vger.kernel.org Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org> Cc: Frank Sorenson <sorenson@redhat.com> Cc: Olga Kornievskaia <okorniev@redhat.com> Cc: Benjamin Coddington <bcodding@redhat.com> Cc: Scott Mayhew <smayhew@redhat.com> Cc: linux-cifs@vger.kernel.org Signed-off-by: Steve French <stfrench@microsoft.com> (cherry picked from commit c5ea306) Signed-off-by: Jonathan Maple <jmaple@ciq.com>
1 parent f8ca85c commit 617fdf8

File tree

8 files changed

+268
-72
lines changed

8 files changed

+268
-72
lines changed

fs/smb/client/cifsglob.h

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@
8686
#define SMB_INTERFACE_POLL_INTERVAL 600
8787

8888
/* maximum number of PDUs in one compound */
89-
#define MAX_COMPOUND 7
89+
#define MAX_COMPOUND 10
9090

9191
/*
9292
* Default number of credits to keep available for SMB3.
@@ -1921,9 +1921,12 @@ static inline bool is_replayable_error(int error)
19211921

19221922

19231923
/* cifs_get_writable_file() flags */
1924-
#define FIND_WR_ANY 0
1925-
#define FIND_WR_FSUID_ONLY 1
1926-
#define FIND_WR_WITH_DELETE 2
1924+
enum cifs_writable_file_flags {
1925+
FIND_WR_ANY = 0U,
1926+
FIND_WR_FSUID_ONLY = (1U << 0),
1927+
FIND_WR_WITH_DELETE = (1U << 1),
1928+
FIND_WR_NO_PENDING_DELETE = (1U << 2),
1929+
};
19271930

19281931
#define MID_FREE 0
19291932
#define MID_REQUEST_ALLOCATED 1
@@ -2357,6 +2360,8 @@ struct smb2_compound_vars {
23572360
struct kvec qi_iov;
23582361
struct kvec io_iov[SMB2_IOCTL_IOV_SIZE];
23592362
struct kvec si_iov[SMB2_SET_INFO_IOV_SIZE];
2363+
struct kvec unlink_iov[SMB2_SET_INFO_IOV_SIZE];
2364+
struct kvec rename_iov[SMB2_SET_INFO_IOV_SIZE];
23602365
struct kvec close_iov;
23612366
struct smb2_file_rename_info rename_info;
23622367
struct smb2_file_link_info link_info;

fs/smb/client/file.c

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -680,7 +680,10 @@ int cifs_open(struct inode *inode, struct file *file)
680680

681681
/* Get the cached handle as SMB2 close is deferred */
682682
if (OPEN_FMODE(file->f_flags) & FMODE_WRITE) {
683-
rc = cifs_get_writable_path(tcon, full_path, FIND_WR_FSUID_ONLY, &cfile);
683+
rc = cifs_get_writable_path(tcon, full_path,
684+
FIND_WR_FSUID_ONLY |
685+
FIND_WR_NO_PENDING_DELETE,
686+
&cfile);
684687
} else {
685688
rc = cifs_get_readable_path(tcon, full_path, &cfile);
686689
}
@@ -2285,6 +2288,9 @@ cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, int flags,
22852288
continue;
22862289
if (with_delete && !(open_file->fid.access & DELETE))
22872290
continue;
2291+
if ((flags & FIND_WR_NO_PENDING_DELETE) &&
2292+
open_file->status_file_deleted)
2293+
continue;
22882294
if (OPEN_FMODE(open_file->f_flags) & FMODE_WRITE) {
22892295
if (!open_file->invalidHandle) {
22902296
/* found a good writable file */
@@ -2402,6 +2408,16 @@ cifs_get_readable_path(struct cifs_tcon *tcon, const char *name,
24022408
spin_unlock(&tcon->open_file_lock);
24032409
free_dentry_path(page);
24042410
*ret_file = find_readable_file(cinode, 0);
2411+
if (*ret_file) {
2412+
spin_lock(&cinode->open_file_lock);
2413+
if ((*ret_file)->status_file_deleted) {
2414+
spin_unlock(&cinode->open_file_lock);
2415+
cifsFileInfo_put(*ret_file);
2416+
*ret_file = NULL;
2417+
} else {
2418+
spin_unlock(&cinode->open_file_lock);
2419+
}
2420+
}
24052421
return *ret_file ? 0 : -ENOENT;
24062422
}
24072423

fs/smb/client/inode.c

Lines changed: 69 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1890,7 +1890,7 @@ cifs_drop_nlink(struct inode *inode)
18901890
* but will return the EACCES to the caller. Note that the VFS does not call
18911891
* unlink on negative dentries currently.
18921892
*/
1893-
int cifs_unlink(struct inode *dir, struct dentry *dentry)
1893+
static int __cifs_unlink(struct inode *dir, struct dentry *dentry, bool sillyrename)
18941894
{
18951895
int rc = 0;
18961896
unsigned int xid;
@@ -1961,7 +1961,11 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry)
19611961
goto psx_del_no_retry;
19621962
}
19631963

1964-
rc = server->ops->unlink(xid, tcon, full_path, cifs_sb, dentry);
1964+
if (sillyrename || (server->vals->protocol_id > SMB10_PROT_ID &&
1965+
d_is_positive(dentry) && d_count(dentry) > 2))
1966+
rc = -EBUSY;
1967+
else
1968+
rc = server->ops->unlink(xid, tcon, full_path, cifs_sb, dentry);
19651969

19661970
psx_del_no_retry:
19671971
if (!rc) {
@@ -2029,6 +2033,11 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry)
20292033
return rc;
20302034
}
20312035

2036+
int cifs_unlink(struct inode *dir, struct dentry *dentry)
2037+
{
2038+
return __cifs_unlink(dir, dentry, false);
2039+
}
2040+
20322041
static int
20332042
cifs_mkdir_qinfo(struct inode *parent, struct dentry *dentry, umode_t mode,
20342043
const char *full_path, struct cifs_sb_info *cifs_sb,
@@ -2316,14 +2325,16 @@ int cifs_rmdir(struct inode *inode, struct dentry *direntry)
23162325
rc = server->ops->rmdir(xid, tcon, full_path, cifs_sb);
23172326
cifs_put_tlink(tlink);
23182327

2328+
cifsInode = CIFS_I(d_inode(direntry));
2329+
23192330
if (!rc) {
2331+
set_bit(CIFS_INO_DELETE_PENDING, &cifsInode->flags);
23202332
spin_lock(&d_inode(direntry)->i_lock);
23212333
i_size_write(d_inode(direntry), 0);
23222334
clear_nlink(d_inode(direntry));
23232335
spin_unlock(&d_inode(direntry)->i_lock);
23242336
}
23252337

2326-
cifsInode = CIFS_I(d_inode(direntry));
23272338
/* force revalidate to go get info when needed */
23282339
cifsInode->time = 0;
23292340

@@ -2416,8 +2427,11 @@ cifs_do_rename(const unsigned int xid, struct dentry *from_dentry,
24162427
}
24172428
#endif /* CONFIG_CIFS_ALLOW_INSECURE_LEGACY */
24182429
do_rename_exit:
2419-
if (rc == 0)
2430+
if (rc == 0) {
24202431
d_move(from_dentry, to_dentry);
2432+
/* Force a new lookup */
2433+
d_drop(from_dentry);
2434+
}
24212435
cifs_put_tlink(tlink);
24222436
return rc;
24232437
}
@@ -2428,6 +2442,7 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
24282442
struct dentry *target_dentry, unsigned int flags)
24292443
{
24302444
const char *from_name, *to_name;
2445+
struct TCP_Server_Info *server;
24312446
void *page1, *page2;
24322447
struct cifs_sb_info *cifs_sb;
24332448
struct tcon_link *tlink;
@@ -2463,6 +2478,7 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
24632478
if (IS_ERR(tlink))
24642479
return PTR_ERR(tlink);
24652480
tcon = tlink_tcon(tlink);
2481+
server = tcon->ses->server;
24662482

24672483
page1 = alloc_dentry_path();
24682484
page2 = alloc_dentry_path();
@@ -2547,19 +2563,53 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
25472563

25482564
unlink_target:
25492565
#endif /* CONFIG_CIFS_ALLOW_INSECURE_LEGACY */
2550-
2551-
/* Try unlinking the target dentry if it's not negative */
2552-
if (d_really_is_positive(target_dentry) && (rc == -EACCES || rc == -EEXIST)) {
2553-
if (d_is_dir(target_dentry))
2554-
tmprc = cifs_rmdir(target_dir, target_dentry);
2555-
else
2556-
tmprc = cifs_unlink(target_dir, target_dentry);
2557-
if (tmprc)
2558-
goto cifs_rename_exit;
2559-
rc = cifs_do_rename(xid, source_dentry, from_name,
2560-
target_dentry, to_name);
2561-
if (!rc)
2562-
rehash = false;
2566+
if (d_really_is_positive(target_dentry)) {
2567+
if (!rc) {
2568+
struct inode *inode = d_inode(target_dentry);
2569+
/*
2570+
* Samba and ksmbd servers allow renaming a target
2571+
* directory that is open, so make sure to update
2572+
* ->i_nlink and then mark it as delete pending.
2573+
*/
2574+
if (S_ISDIR(inode->i_mode)) {
2575+
drop_cached_dir_by_name(xid, tcon, to_name, cifs_sb);
2576+
spin_lock(&inode->i_lock);
2577+
i_size_write(inode, 0);
2578+
clear_nlink(inode);
2579+
spin_unlock(&inode->i_lock);
2580+
set_bit(CIFS_INO_DELETE_PENDING, &CIFS_I(inode)->flags);
2581+
CIFS_I(inode)->time = 0; /* force reval */
2582+
inode_set_ctime_current(inode);
2583+
inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
2584+
}
2585+
} else if (rc == -EACCES || rc == -EEXIST) {
2586+
/*
2587+
* Rename failed, possibly due to a busy target.
2588+
* Retry it by unliking the target first.
2589+
*/
2590+
if (d_is_dir(target_dentry)) {
2591+
tmprc = cifs_rmdir(target_dir, target_dentry);
2592+
} else {
2593+
tmprc = __cifs_unlink(target_dir, target_dentry,
2594+
server->vals->protocol_id > SMB10_PROT_ID);
2595+
}
2596+
if (tmprc) {
2597+
/*
2598+
* Some servers will return STATUS_ACCESS_DENIED
2599+
* or STATUS_DIRECTORY_NOT_EMPTY when failing to
2600+
* rename a non-empty directory. Make sure to
2601+
* propagate the appropriate error back to
2602+
* userspace.
2603+
*/
2604+
if (tmprc == -EEXIST || tmprc == -ENOTEMPTY)
2605+
rc = tmprc;
2606+
goto cifs_rename_exit;
2607+
}
2608+
rc = cifs_do_rename(xid, source_dentry, from_name,
2609+
target_dentry, to_name);
2610+
if (!rc)
2611+
rehash = false;
2612+
}
25632613
}
25642614

25652615
/* force revalidate to go get info when needed */
@@ -2588,6 +2638,8 @@ cifs_dentry_needs_reval(struct dentry *dentry)
25882638
struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
25892639
struct cached_fid *cfid = NULL;
25902640

2641+
if (test_bit(CIFS_INO_DELETE_PENDING, &cifs_i->flags))
2642+
return false;
25912643
if (cifs_i->time == 0)
25922644
return true;
25932645

fs/smb/client/smb2glob.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,9 @@ enum smb2_compound_ops {
3030
SMB2_OP_QUERY_DIR,
3131
SMB2_OP_MKDIR,
3232
SMB2_OP_RENAME,
33-
SMB2_OP_DELETE,
3433
SMB2_OP_HARDLINK,
3534
SMB2_OP_SET_EOF,
36-
SMB2_OP_RMDIR,
35+
SMB2_OP_UNLINK,
3736
SMB2_OP_POSIX_QUERY_INFO,
3837
SMB2_OP_SET_REPARSE,
3938
SMB2_OP_GET_REPARSE,

0 commit comments

Comments
 (0)