reiserfs: fix race in readdir
PATCH 1/1 reiserfs: fix race in readdir
jdm-20004 reiserfs_delete_xattrs: Couldn't delete all xattrs (-2)
The -ENOENT is due to readdir calling dir_emit on the same entry twice.
If the dir_emit callback sleeps and the tree is changed underneath us, we won't be able to trust deh_offset(deh) anymore. We need to save next_pos before we might sleep so we can find the next entry.
This can also affect non-xattr users of readdir, though the race is tighter.
--- fs/reiserfs/dir.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
fs/reiserfs/dir.c +7 -2
@@ -179,6 +179,13 @@ int reiserfs_readdir_inode(struct inodememcpy (local_buf, d_name, d_reclen);/* +* deh_offset(deh) will be invalid if dir_emit +* sleeps. We need to know the offset after +* this one to continue. +*/ + next_pos =deh_offset (deh) +1 ; + +/* * Since filldir might sleep, we can release * the write lock here for other waiters */
@@ -196,8 +203,6 @@ int reiserfs_readdir_inode(struct inodeif (local_buf != small_buf) {kfree (local_buf); } -// next entry should be looked for with such offset - next_pos =deh_offset (deh) +1 ;if (item_moved (&tmp_ih, &path_to_entry)) { set_cpu_key_k_offset(&pos_key,
Signature · 2 lines
Jeff Mahoney SUSE Labs
Quoted 4 lines · Jeff Mahoney
jdm-20004
reiserfs_delete_xattrs: Couldn't delete all xattrs (-2)The
-ENOENTis due to readdir callingdir_emiton the same entry twice.
Quoted 8 lines
If the
dir_emitcallback sleeps and the tree is changed underneath us, we won't be able to trustdeh_offset(deh)anymore. We need to savenext_posbefore we might sleep so we can find the next entry.This can also affect non-xattr users of readdir, though the race is tighter.
Cc <[email protected]>Signed-off-by Jeff Mahoney <[email protected]>
The patch looks good to me so unless someone objects, I'll merge this
patch with Linus.
Honza
Quoted 33 lines
--- fs/reiserfs/dir.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)fs/reiserfs/dir.c +7
-2@@ -179,6 +179,13 @@ int reiserfs_readdir_inode(struct inodememcpy (local_buf, d_name, d_reclen);/* +* deh_offset(deh) will be invalid if dir_emit +* sleeps. We need to know the offset after +* this one to continue. +*/ + next_pos =deh_offset (deh) +1 ; + +/* * Since filldir might sleep, we can release * the write lock here for other waiters */ @@ -196,8 +203,6 @@ int reiserfs_readdir_inode(struct inodeif (local_buf != small_buf) {kfree (local_buf); } -// next entry should be looked for with such offset - next_pos =deh_offset (deh) +1 ;if (item_moved (&tmp_ih, &path_to_entry)) { set_cpu_key_k_offset(&pos_key,Signature · 3 lines
-- Jeff Mahoney SUSE Labs
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
Quoted 9 lines · Jan Kara
Quoted 8 lines · Jeff Mahoney
jdm-20004
reiserfs_delete_xattrs: Couldn't delete all xattrs (-2)The
-ENOENTis due to readdir callingdir_emiton the same entry twice.If the
dir_emitcallback sleeps and the tree is changed underneath us, we won't be able to trustdeh_offset(deh)anymore. We need to savenext_posbefore we might sleep so we can find the next entry.
Quoted 6 lines
Quoted 4 lines
This can also affect non-xattr users of readdir, though the race is tighter.
Cc <[email protected]>Signed-off-by Jeff Mahoney <[email protected]>The patch looks good to me so unless someone objects, I'll merge this
patch with Linus.
Umm, actually, what if dir_emit() returns error? Previously we tried
again the same entry, now we continue with the next one. Is that intended? If yes, please comment on that in the changelog.
Honza
Quoted 36 lines
Quoted 33 lines
--- fs/reiserfs/dir.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)fs/reiserfs/dir.c +7
-2@@ -179,6 +179,13 @@ int reiserfs_readdir_inode(struct inodememcpy (local_buf, d_name, d_reclen);/* +* deh_offset(deh) will be invalid if dir_emit +* sleeps. We need to know the offset after +* this one to continue. +*/ + next_pos =deh_offset (deh) +1 ; + +/* * Since filldir might sleep, we can release * the write lock here for other waiters */ @@ -196,8 +203,6 @@ int reiserfs_readdir_inode(struct inodeif (local_buf != small_buf) {kfree (local_buf); } -// next entry should be looked for with such offset - next_pos =deh_offset (deh) +1 ;if (item_moved (&tmp_ih, &path_to_entry)) { set_cpu_key_k_offset(&pos_key,Signature · 3 lines
-- Jeff Mahoney SUSE LabsSignature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 12 lines · Jan Kara
Quoted 11 lines · Jan Kara
Quoted 10 lines · Jeff Mahoney
jdm-20004
reiserfs_delete_xattrs: Couldn't delete all xattrs (-2)The
-ENOENTis due to readdir callingdir_emiton the same entry twice.If the
dir_emitcallback sleeps and the tree is changed underneath us, we won't be able to trustdeh_offset(deh)anymore. We need to savenext_posbefore we might sleep so we can find the next entry.This can also affect non-xattr users of readdir, though the race is tighter.
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
Cc <[email protected]>Signed-off-by Jeff Mahoney <[email protected]>The patch looks good to me so unless someone objects, I'll merge this
patch with Linus.Umm, actually, what if
dir_emit()returns error? Previously we tried
again the same entry, now we continue with the next one. Is that intended? If yes, please comment on that in the changelog.
Sigh. Yes. You're right. I missed that part of the error handling.
- -Jeff
Quoted 30 lines
Quoted 30 lines
Quoted 30 lines
--- fs/reiserfs/dir.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)fs/reiserfs/dir.c +7
-2@@ -179,6 +179,13 @@ int reiserfs_readdir_inode(struct inodememcpy (local_buf, d_name, d_reclen);/* +* deh_offset(deh) will be invalid if dir_emit +* sleeps. We need to know the offset after +* this one to continue. +*/ + next_pos =deh_offset (deh) +1 ; + +/* * Since filldir might sleep, we can release * the write lock here for other waiters */ @@ -196,8 +203,6 @@ int reiserfs_readdir_inode(struct inodeif (local_buf != small_buf) {kfree (local_buf); } -// next entry should be looked for with such offset - next_pos =deh_offset (deh) +1 ;if (item_moved (&tmp_ih, &path_to_entry)) { set_cpu_key_k_offset(&pos_key,
Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
Signature · 2 lines
Jeff Mahoney SUSE LabsSignature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
- --
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.19 (Darwin)
iQIcBAEBAgAGBQJTPFg6AAoJEB57S2MheeWyNIoQAI9ZbPx1i2ycEfWV6qSzFH+/ L/4OZy4pVM8U4j0ljDlhvrRiKQ/r3843enKKSrpsJawIbWwXiOVY1Zvu3+FZ0T8Q Bc+BL0gH43H4UTKh9EsAX0CfG9CGau7sFC1meOBPEs27OBMPn8Xgbg4zs4nwlaDz TLo166LcdV55JYcndNzt/YIwOaRK6roTEJg2+5uX+hM0ZGDZan4+AMHyjdqV4VTk JG8K/mthPF/tgZQMSHSIh3Xg9Gth3gPaPBsr0M2SdDxVQcao7BiCNgUvRMXyOQOl 2uVkzuX2fC5JYM569WyPYhNXNR/IQwgtJMRYkoVvtu/oMk5dhLQTeDLl+A4BrYFY ucLG1VxY0gd215+vW/0hXjnBp5Gm9jtXeFiCWeFMBHWhvVpxHlcXfwEhEHLBPBJC Dc48sJLqFWYtqM/WZ6eryUIdWgJsup/z8vEMCM18P+QUeG3DHANmACt93XptWw4b 8I6LyqisYOqKk1rZ0QDvdo4zq6ZHqQwbPEGmqOLnA2tbn+iYxGk5WGyWDh1I0SbW RQowfNhvJKjj7qzb5C+GMfE04OtGw1Qa6JoR/x+KzAWafjRstSpMuupCoHg5sj1M xEQka9U8vYZtnbxwzSl0yRTMLzZ3YmrLNjV0L9AmqKVm7H5BS6vscEDwQtV9fH1s n4YTCcGORZSzvX7YNP8m
=xFMg
-----END PGP SIGNATURE-----