Skip to content

Commit f4cd666

Browse files
torvaldstorvalds
committed
Merge tag 'afs-fixes-20200413' of git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs
Pull AFS fixes from David Howells: - Fix the decoding of fetched file status records so that the xdr pointer is advanced under all circumstances. - Fix the decoding of a fetched file status record that indicates an inline abort (ie. an error) so that it sets the flag saying the decoder stored the abort code. - Fix the decoding of the result of the rename operation so that it doesn't skip the decoding of the second fetched file status (ie. that of the dest dir) in the case that the source and dest dirs were the same as this causes the xdr pointer not to be advanced, leading to incorrect decoding of subsequent parts of the reply. - Fix the dump of a bad YFSFetchStatus record to dump the full length. - Fix a race between local editing of directory contents and accessing the dir for reading or d_revalidate by using the same lock in both. - Fix afs_d_revalidate() to not accidentally reverse the version on a dentry when it's meant to be bringing it forward. * tag 'afs-fixes-20200413' of git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs: afs: Fix afs_d_validate() to set the right directory version afs: Fix race between post-modification dir edit and readdir/d_revalidate afs: Fix length of dump of bad YFSFetchStatus record afs: Fix rename operation status delivery afs: Fix decoding of inline abort codes from version 1 status records afs: Fix missing XDR advance in xdr_decode_{AFS,YFS}FSFetchStatus()
2 parents ac4075b + 40fc810 commit f4cd666

File tree

4 files changed

+112
-71
lines changed

4 files changed

+112
-71
lines changed

‎fs/afs/dir.c

Lines changed: 67 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,7 +1032,7 @@ static int afs_d_revalidate(struct dentry *dentry, unsigned int flags)
10321032
struct dentry *parent;
10331033
struct inode *inode;
10341034
struct key *key;
1035-
afs_dataversion_t dir_version;
1035+
afs_dataversion_t dir_version, invalid_before;
10361036
long de_version;
10371037
int ret;
10381038

@@ -1084,8 +1084,8 @@ static int afs_d_revalidate(struct dentry *dentry, unsigned int flags)
10841084
if (de_version == (long)dir_version)
10851085
goto out_valid_noupdate;
10861086

1087-
dir_version = dir->invalid_before;
1088-
if (de_version - (long)dir_version >= 0)
1087+
invalid_before = dir->invalid_before;
1088+
if (de_version - (long)invalid_before >= 0)
10891089
goto out_valid;
10901090

10911091
_debug("dir modified");
@@ -1275,6 +1275,7 @@ static int afs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
12751275
struct afs_fs_cursor fc;
12761276
struct afs_vnode *dvnode = AFS_FS_I(dir);
12771277
struct key *key;
1278+
afs_dataversion_t data_version;
12781279
int ret;
12791280

12801281
mode |= S_IFDIR;
@@ -1295,7 +1296,7 @@ static int afs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
12951296

12961297
ret = -ERESTARTSYS;
12971298
if (afs_begin_vnode_operation(&fc, dvnode, key, true)) {
1298-
afs_dataversion_t data_version = dvnode->status.data_version + 1;
1299+
data_version = dvnode->status.data_version + 1;
12991300

13001301
while (afs_select_fileserver(&fc)) {
13011302
fc.cb_break = afs_calc_vnode_cb_break(dvnode);
@@ -1316,10 +1317,14 @@ static int afs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
13161317
goto error_key;
13171318
}
13181319

1319-
if (ret == 0 &&
1320-
test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags))
1321-
afs_edit_dir_add(dvnode, &dentry->d_name, &iget_data.fid,
1322-
afs_edit_dir_for_create);
1320+
if (ret == 0) {
1321+
down_write(&dvnode->validate_lock);
1322+
if (test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags) &&
1323+
dvnode->status.data_version == data_version)
1324+
afs_edit_dir_add(dvnode, &dentry->d_name, &iget_data.fid,
1325+
afs_edit_dir_for_create);
1326+
up_write(&dvnode->validate_lock);
1327+
}
13231328

13241329
key_put(key);
13251330
kfree(scb);
@@ -1360,6 +1365,7 @@ static int afs_rmdir(struct inode *dir, struct dentry *dentry)
13601365
struct afs_fs_cursor fc;
13611366
struct afs_vnode *dvnode = AFS_FS_I(dir), *vnode = NULL;
13621367
struct key *key;
1368+
afs_dataversion_t data_version;
13631369
int ret;
13641370

13651371
_enter("{%llx:%llu},{%pd}",
@@ -1391,7 +1397,7 @@ static int afs_rmdir(struct inode *dir, struct dentry *dentry)
13911397

13921398
ret = -ERESTARTSYS;
13931399
if (afs_begin_vnode_operation(&fc, dvnode, key, true)) {
1394-
afs_dataversion_t data_version = dvnode->status.data_version + 1;
1400+
data_version = dvnode->status.data_version + 1;
13951401

13961402
while (afs_select_fileserver(&fc)) {
13971403
fc.cb_break = afs_calc_vnode_cb_break(dvnode);
@@ -1404,9 +1410,12 @@ static int afs_rmdir(struct inode *dir, struct dentry *dentry)
14041410
ret = afs_end_vnode_operation(&fc);
14051411
if (ret == 0) {
14061412
afs_dir_remove_subdir(dentry);
1407-
if (test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags))
1413+
down_write(&dvnode->validate_lock);
1414+
if (test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags) &&
1415+
dvnode->status.data_version == data_version)
14081416
afs_edit_dir_remove(dvnode, &dentry->d_name,
14091417
afs_edit_dir_for_rmdir);
1418+
up_write(&dvnode->validate_lock);
14101419
}
14111420
}
14121421

@@ -1544,10 +1553,15 @@ static int afs_unlink(struct inode *dir, struct dentry *dentry)
15441553
ret = afs_end_vnode_operation(&fc);
15451554
if (ret == 0 && !(scb[1].have_status || scb[1].have_error))
15461555
ret = afs_dir_remove_link(dvnode, dentry, key);
1547-
if (ret == 0 &&
1548-
test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags))
1549-
afs_edit_dir_remove(dvnode, &dentry->d_name,
1550-
afs_edit_dir_for_unlink);
1556+
1557+
if (ret == 0) {
1558+
down_write(&dvnode->validate_lock);
1559+
if (test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags) &&
1560+
dvnode->status.data_version == data_version)
1561+
afs_edit_dir_remove(dvnode, &dentry->d_name,
1562+
afs_edit_dir_for_unlink);
1563+
up_write(&dvnode->validate_lock);
1564+
}
15511565
}
15521566

15531567
if (need_rehash && ret < 0 && ret != -ENOENT)
@@ -1573,6 +1587,7 @@ static int afs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
15731587
struct afs_status_cb *scb;
15741588
struct afs_vnode *dvnode = AFS_FS_I(dir);
15751589
struct key *key;
1590+
afs_dataversion_t data_version;
15761591
int ret;
15771592

15781593
mode |= S_IFREG;
@@ -1597,7 +1612,7 @@ static int afs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
15971612

15981613
ret = -ERESTARTSYS;
15991614
if (afs_begin_vnode_operation(&fc, dvnode, key, true)) {
1600-
afs_dataversion_t data_version = dvnode->status.data_version + 1;
1615+
data_version = dvnode->status.data_version + 1;
16011616

16021617
while (afs_select_fileserver(&fc)) {
16031618
fc.cb_break = afs_calc_vnode_cb_break(dvnode);
@@ -1618,9 +1633,12 @@ static int afs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
16181633
goto error_key;
16191634
}
16201635

1621-
if (test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags))
1636+
down_write(&dvnode->validate_lock);
1637+
if (test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags) &&
1638+
dvnode->status.data_version == data_version)
16221639
afs_edit_dir_add(dvnode, &dentry->d_name, &iget_data.fid,
16231640
afs_edit_dir_for_create);
1641+
up_write(&dvnode->validate_lock);
16241642

16251643
kfree(scb);
16261644
key_put(key);
@@ -1648,6 +1666,7 @@ static int afs_link(struct dentry *from, struct inode *dir,
16481666
struct afs_vnode *dvnode = AFS_FS_I(dir);
16491667
struct afs_vnode *vnode = AFS_FS_I(d_inode(from));
16501668
struct key *key;
1669+
afs_dataversion_t data_version;
16511670
int ret;
16521671

16531672
_enter("{%llx:%llu},{%llx:%llu},{%pd}",
@@ -1672,7 +1691,7 @@ static int afs_link(struct dentry *from, struct inode *dir,
16721691

16731692
ret = -ERESTARTSYS;
16741693
if (afs_begin_vnode_operation(&fc, dvnode, key, true)) {
1675-
afs_dataversion_t data_version = dvnode->status.data_version + 1;
1694+
data_version = dvnode->status.data_version + 1;
16761695

16771696
if (mutex_lock_interruptible_nested(&vnode->io_lock, 1) < 0) {
16781697
afs_end_vnode_operation(&fc);
@@ -1702,9 +1721,12 @@ static int afs_link(struct dentry *from, struct inode *dir,
17021721
goto error_key;
17031722
}
17041723

1705-
if (test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags))
1724+
down_write(&dvnode->validate_lock);
1725+
if (test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags) &&
1726+
dvnode->status.data_version == data_version)
17061727
afs_edit_dir_add(dvnode, &dentry->d_name, &vnode->fid,
17071728
afs_edit_dir_for_link);
1729+
up_write(&dvnode->validate_lock);
17081730

17091731
key_put(key);
17101732
kfree(scb);
@@ -1732,6 +1754,7 @@ static int afs_symlink(struct inode *dir, struct dentry *dentry,
17321754
struct afs_status_cb *scb;
17331755
struct afs_vnode *dvnode = AFS_FS_I(dir);
17341756
struct key *key;
1757+
afs_dataversion_t data_version;
17351758
int ret;
17361759

17371760
_enter("{%llx:%llu},{%pd},%s",
@@ -1759,7 +1782,7 @@ static int afs_symlink(struct inode *dir, struct dentry *dentry,
17591782

17601783
ret = -ERESTARTSYS;
17611784
if (afs_begin_vnode_operation(&fc, dvnode, key, true)) {
1762-
afs_dataversion_t data_version = dvnode->status.data_version + 1;
1785+
data_version = dvnode->status.data_version + 1;
17631786

17641787
while (afs_select_fileserver(&fc)) {
17651788
fc.cb_break = afs_calc_vnode_cb_break(dvnode);
@@ -1780,9 +1803,12 @@ static int afs_symlink(struct inode *dir, struct dentry *dentry,
17801803
goto error_key;
17811804
}
17821805

1783-
if (test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags))
1806+
down_write(&dvnode->validate_lock);
1807+
if (test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags) &&
1808+
dvnode->status.data_version == data_version)
17841809
afs_edit_dir_add(dvnode, &dentry->d_name, &iget_data.fid,
17851810
afs_edit_dir_for_symlink);
1811+
up_write(&dvnode->validate_lock);
17861812

17871813
key_put(key);
17881814
kfree(scb);
@@ -1812,6 +1838,8 @@ static int afs_rename(struct inode *old_dir, struct dentry *old_dentry,
18121838
struct dentry *tmp = NULL, *rehash = NULL;
18131839
struct inode *new_inode;
18141840
struct key *key;
1841+
afs_dataversion_t orig_data_version;
1842+
afs_dataversion_t new_data_version;
18151843
bool new_negative = d_is_negative(new_dentry);
18161844
int ret;
18171845

@@ -1890,10 +1918,6 @@ static int afs_rename(struct inode *old_dir, struct dentry *old_dentry,
18901918

18911919
ret = -ERESTARTSYS;
18921920
if (afs_begin_vnode_operation(&fc, orig_dvnode, key, true)) {
1893-
afs_dataversion_t orig_data_version;
1894-
afs_dataversion_t new_data_version;
1895-
struct afs_status_cb *new_scb = &scb[1];
1896-
18971921
orig_data_version = orig_dvnode->status.data_version + 1;
18981922

18991923
if (orig_dvnode != new_dvnode) {
@@ -1904,15 +1928,14 @@ static int afs_rename(struct inode *old_dir, struct dentry *old_dentry,
19041928
new_data_version = new_dvnode->status.data_version + 1;
19051929
} else {
19061930
new_data_version = orig_data_version;
1907-
new_scb = &scb[0];
19081931
}
19091932

19101933
while (afs_select_fileserver(&fc)) {
19111934
fc.cb_break = afs_calc_vnode_cb_break(orig_dvnode);
19121935
fc.cb_break_2 = afs_calc_vnode_cb_break(new_dvnode);
19131936
afs_fs_rename(&fc, old_dentry->d_name.name,
19141937
new_dvnode, new_dentry->d_name.name,
1915-
&scb[0], new_scb);
1938+
&scb[0], &scb[1]);
19161939
}
19171940

19181941
afs_vnode_commit_status(&fc, orig_dvnode, fc.cb_break,
@@ -1930,18 +1953,25 @@ static int afs_rename(struct inode *old_dir, struct dentry *old_dentry,
19301953
if (ret == 0) {
19311954
if (rehash)
19321955
d_rehash(rehash);
1933-
if (test_bit(AFS_VNODE_DIR_VALID, &orig_dvnode->flags))
1934-
afs_edit_dir_remove(orig_dvnode, &old_dentry->d_name,
1935-
afs_edit_dir_for_rename_0);
1956+
down_write(&orig_dvnode->validate_lock);
1957+
if (test_bit(AFS_VNODE_DIR_VALID, &orig_dvnode->flags) &&
1958+
orig_dvnode->status.data_version == orig_data_version)
1959+
afs_edit_dir_remove(orig_dvnode, &old_dentry->d_name,
1960+
afs_edit_dir_for_rename_0);
1961+
if (orig_dvnode != new_dvnode) {
1962+
up_write(&orig_dvnode->validate_lock);
19361963

1937-
if (!new_negative &&
1938-
test_bit(AFS_VNODE_DIR_VALID, &new_dvnode->flags))
1939-
afs_edit_dir_remove(new_dvnode, &new_dentry->d_name,
1940-
afs_edit_dir_for_rename_1);
1964+
down_write(&new_dvnode->validate_lock);
1965+
}
1966+
if (test_bit(AFS_VNODE_DIR_VALID, &new_dvnode->flags) &&
1967+
orig_dvnode->status.data_version == new_data_version) {
1968+
if (!new_negative)
1969+
afs_edit_dir_remove(new_dvnode, &new_dentry->d_name,
1970+
afs_edit_dir_for_rename_1);
19411971

1942-
if (test_bit(AFS_VNODE_DIR_VALID, &new_dvnode->flags))
19431972
afs_edit_dir_add(new_dvnode, &new_dentry->d_name,
19441973
&vnode->fid, afs_edit_dir_for_rename_2);
1974+
}
19451975

19461976
new_inode = d_inode(new_dentry);
19471977
if (new_inode) {
@@ -1957,14 +1987,10 @@ static int afs_rename(struct inode *old_dir, struct dentry *old_dentry,
19571987
* Note that if we ever implement RENAME_EXCHANGE, we'll have
19581988
* to update both dentries with opposing dir versions.
19591989
*/
1960-
if (new_dvnode != orig_dvnode) {
1961-
afs_update_dentry_version(&fc, old_dentry, &scb[1]);
1962-
afs_update_dentry_version(&fc, new_dentry, &scb[1]);
1963-
} else {
1964-
afs_update_dentry_version(&fc, old_dentry, &scb[0]);
1965-
afs_update_dentry_version(&fc, new_dentry, &scb[0]);
1966-
}
1990+
afs_update_dentry_version(&fc, old_dentry, &scb[1]);
1991+
afs_update_dentry_version(&fc, new_dentry, &scb[1]);
19671992
d_move(old_dentry, new_dentry);
1993+
up_write(&new_dvnode->validate_lock);
19681994
goto error_tmp;
19691995
}
19701996

‎fs/afs/dir_silly.c

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ static int afs_do_silly_rename(struct afs_vnode *dvnode, struct afs_vnode *vnode
2121
{
2222
struct afs_fs_cursor fc;
2323
struct afs_status_cb *scb;
24+
afs_dataversion_t dir_data_version;
2425
int ret = -ERESTARTSYS;
2526

2627
_enter("%pd,%pd", old, new);
@@ -31,7 +32,7 @@ static int afs_do_silly_rename(struct afs_vnode *dvnode, struct afs_vnode *vnode
3132

3233
trace_afs_silly_rename(vnode, false);
3334
if (afs_begin_vnode_operation(&fc, dvnode, key, true)) {
34-
afs_dataversion_t dir_data_version = dvnode->status.data_version + 1;
35+
dir_data_version = dvnode->status.data_version + 1;
3536

3637
while (afs_select_fileserver(&fc)) {
3738
fc.cb_break = afs_calc_vnode_cb_break(dvnode);
@@ -54,12 +55,15 @@ static int afs_do_silly_rename(struct afs_vnode *dvnode, struct afs_vnode *vnode
5455
dvnode->silly_key = key_get(key);
5556
}
5657

57-
if (test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags))
58+
down_write(&dvnode->validate_lock);
59+
if (test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags) &&
60+
dvnode->status.data_version == dir_data_version) {
5861
afs_edit_dir_remove(dvnode, &old->d_name,
5962
afs_edit_dir_for_silly_0);
60-
if (test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags))
6163
afs_edit_dir_add(dvnode, &new->d_name,
6264
&vnode->fid, afs_edit_dir_for_silly_1);
65+
}
66+
up_write(&dvnode->validate_lock);
6367
}
6468

6569
kfree(scb);
@@ -181,10 +185,14 @@ static int afs_do_silly_unlink(struct afs_vnode *dvnode, struct afs_vnode *vnode
181185
clear_bit(AFS_VNODE_CB_PROMISED, &vnode->flags);
182186
}
183187
}
184-
if (ret == 0 &&
185-
test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags))
186-
afs_edit_dir_remove(dvnode, &dentry->d_name,
187-
afs_edit_dir_for_unlink);
188+
if (ret == 0) {
189+
down_write(&dvnode->validate_lock);
190+
if (test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags) &&
191+
dvnode->status.data_version == dir_data_version)
192+
afs_edit_dir_remove(dvnode, &dentry->d_name,
193+
afs_edit_dir_for_unlink);
194+
up_write(&dvnode->validate_lock);
195+
}
188196
}
189197

190198
kfree(scb);

0 commit comments

Comments
 (0)