Commit 2d7f2ea9 authored by Al Viro's avatar Al Viro Committed by Linus Torvalds

[PATCH] Fix ext2 readdir f_pos re-validation logic

This fixes not one, but _two_, silly (but admittedly hard to hit) bugs
in the ext2 filesystem "readdir()" function.  It also cleans up the code
to avoid the unnecessary goto mess.

The bugs were related to re-valiating the f_pos value after somebody had
either done an "lseek()" on the directory to an invalid offset, or when
the offset had become invalid due to a file being unlinked in the
directory.  The code would not only set the f_version too eagerly, it
would also not update f_pos appropriately for when the offset fixup took
place.

When that happened, we'd occasionally subsequently fail the readdir()
even when we shouldn't (no real harm done, but an ugly printk, and
obviously you would end up not necessarily seeing all entries).

Thanks to Masoud Sharbiani <masouds@google.com> who noticed the problem
and had a test-case for it, and also fixed up a thinko in the first
version of this patch.
Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
Acked-by: default avatarMasoud Sharbiani <masouds@google.com>
Signed-off-by: default avatarLinus Torvalds <torvalds@osdl.org>
parent f13b8358
...@@ -256,11 +256,10 @@ ext2_readdir (struct file * filp, void * dirent, filldir_t filldir) ...@@ -256,11 +256,10 @@ ext2_readdir (struct file * filp, void * dirent, filldir_t filldir)
unsigned long npages = dir_pages(inode); unsigned long npages = dir_pages(inode);
unsigned chunk_mask = ~(ext2_chunk_size(inode)-1); unsigned chunk_mask = ~(ext2_chunk_size(inode)-1);
unsigned char *types = NULL; unsigned char *types = NULL;
int need_revalidate = (filp->f_version != inode->i_version); int need_revalidate = filp->f_version != inode->i_version;
int ret;
if (pos > inode->i_size - EXT2_DIR_REC_LEN(1)) if (pos > inode->i_size - EXT2_DIR_REC_LEN(1))
goto success; return 0;
if (EXT2_HAS_INCOMPAT_FEATURE(sb, EXT2_FEATURE_INCOMPAT_FILETYPE)) if (EXT2_HAS_INCOMPAT_FEATURE(sb, EXT2_FEATURE_INCOMPAT_FILETYPE))
types = ext2_filetype_table; types = ext2_filetype_table;
...@@ -275,12 +274,15 @@ ext2_readdir (struct file * filp, void * dirent, filldir_t filldir) ...@@ -275,12 +274,15 @@ ext2_readdir (struct file * filp, void * dirent, filldir_t filldir)
"bad page in #%lu", "bad page in #%lu",
inode->i_ino); inode->i_ino);
filp->f_pos += PAGE_CACHE_SIZE - offset; filp->f_pos += PAGE_CACHE_SIZE - offset;
ret = -EIO; return -EIO;
goto done;
} }
kaddr = page_address(page); kaddr = page_address(page);
if (need_revalidate) { if (unlikely(need_revalidate)) {
if (offset) {
offset = ext2_validate_entry(kaddr, offset, chunk_mask); offset = ext2_validate_entry(kaddr, offset, chunk_mask);
filp->f_pos = (n<<PAGE_CACHE_SHIFT) + offset;
}
filp->f_version = inode->i_version;
need_revalidate = 0; need_revalidate = 0;
} }
de = (ext2_dirent *)(kaddr+offset); de = (ext2_dirent *)(kaddr+offset);
...@@ -289,9 +291,8 @@ ext2_readdir (struct file * filp, void * dirent, filldir_t filldir) ...@@ -289,9 +291,8 @@ ext2_readdir (struct file * filp, void * dirent, filldir_t filldir)
if (de->rec_len == 0) { if (de->rec_len == 0) {
ext2_error(sb, __FUNCTION__, ext2_error(sb, __FUNCTION__,
"zero-length directory entry"); "zero-length directory entry");
ret = -EIO;
ext2_put_page(page); ext2_put_page(page);
goto done; return -EIO;
} }
if (de->inode) { if (de->inode) {
int over; int over;
...@@ -306,19 +307,14 @@ ext2_readdir (struct file * filp, void * dirent, filldir_t filldir) ...@@ -306,19 +307,14 @@ ext2_readdir (struct file * filp, void * dirent, filldir_t filldir)
le32_to_cpu(de->inode), d_type); le32_to_cpu(de->inode), d_type);
if (over) { if (over) {
ext2_put_page(page); ext2_put_page(page);
goto success; return 0;
} }
} }
filp->f_pos += le16_to_cpu(de->rec_len); filp->f_pos += le16_to_cpu(de->rec_len);
} }
ext2_put_page(page); ext2_put_page(page);
} }
return 0;
success:
ret = 0;
done:
filp->f_version = inode->i_version;
return ret;
} }
/* /*
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment