Patchwork [2/2] ext4: fix readdir error in case inline_data+^dir_index.

login
register
mail settings
Submitter Tao Ma
Date April 10, 2013, 3:37 p.m.
Message ID <1365608228-3950-2-git-send-email-tm@tao.ma>
Download mbox | patch
Permalink /patch/235410/
State Accepted
Headers show

Comments

Tao Ma - April 10, 2013, 3:37 p.m.
From: Tao Ma <boyu.mt@taobao.com>

Zach reported a problem that if inline data is enabled, we don't
tell the difference between the offset of '.' and '..'. And a
getdents will fail if the user only want to get '.'. And what's
worse, we may meet with duplicate dir entries as the offset
for inline dir and non-inline one is quite different.

This patch just try to resolve this problem if dir_index
is disabled. In this case, f_pos is the real offset with
the dir block, so for inline dir, we just pretend as if
we are a dir block and returns the offset like a norml
dir block does.

Reported-by: Zach Brown <zab@redhat.com>
Signed-off-by: Tao Ma <boyu.mt@taobao.com>
---
 fs/ext4/inline.c |   69 +++++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 51 insertions(+), 18 deletions(-)
Theodore Ts'o - April 19, 2013, 10 p.m.
On Wed, Apr 10, 2013 at 11:37:08PM +0800, Tao Ma wrote:
> From: Tao Ma <boyu.mt@taobao.com>
> 
> Zach reported a problem that if inline data is enabled, we don't
> tell the difference between the offset of '.' and '..'. And a
> getdents will fail if the user only want to get '.'. And what's
> worse, we may meet with duplicate dir entries as the offset
> for inline dir and non-inline one is quite different.
> 
> This patch just try to resolve this problem if dir_index
> is disabled. In this case, f_pos is the real offset with
> the dir block, so for inline dir, we just pretend as if
> we are a dir block and returns the offset like a norml
> dir block does.
> 
> Reported-by: Zach Brown <zab@redhat.com>
> Signed-off-by: Tao Ma <boyu.mt@taobao.com>

Thanks, applied.

					- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index abf8b62..3e2bf87 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -1396,6 +1396,14 @@  out:
 	return ret;
 }
 
+/*
+ * So this function is called when the volume is mkfsed with
+ * dir_index disabled. In order to keep f_pos persistent
+ * after we convert from an inlined dir to a blocked based,
+ * we just pretend that we are a normal dir and return the
+ * offset as if '.' and '..' really take place.
+ *
+ */
 int ext4_read_inline_dir(struct file *filp,
 			 void *dirent, filldir_t filldir,
 			 int *has_inline_data)
@@ -1409,6 +1417,7 @@  int ext4_read_inline_dir(struct file *filp,
 	int ret, inline_size = 0;
 	struct ext4_iloc iloc;
 	void *dir_buf = NULL;
+	int dotdot_offset, dotdot_size, extra_offset, extra_size;
 
 	ret = ext4_get_inode_loc(inode, &iloc);
 	if (ret)
@@ -1437,8 +1446,21 @@  int ext4_read_inline_dir(struct file *filp,
 	sb = inode->i_sb;
 	stored = 0;
 	parent_ino = le32_to_cpu(((struct ext4_dir_entry_2 *)dir_buf)->inode);
+	offset = filp->f_pos;
+
+	/*
+	 * dotdot_offset and dotdot_size is the real offset and
+	 * size for ".." and "." if the dir is block based while
+	 * the real size for them are only EXT4_INLINE_DOTDOT_SIZE.
+	 * So we will use extra_offset and extra_size to indicate them
+	 * during the inline dir iteration.
+	 */
+	dotdot_offset = EXT4_DIR_REC_LEN(1);
+	dotdot_size = dotdot_offset + EXT4_DIR_REC_LEN(2);
+	extra_offset = dotdot_size - EXT4_INLINE_DOTDOT_SIZE;
+	extra_size = extra_offset + inline_size;
 
-	while (!error && !stored && filp->f_pos < inode->i_size) {
+	while (!error && !stored && filp->f_pos < extra_size) {
 revalidate:
 		/*
 		 * If the version has changed since the last call to
@@ -1447,15 +1469,23 @@  revalidate:
 		 * dir to make sure.
 		 */
 		if (filp->f_version != inode->i_version) {
-			for (i = 0;
-			     i < inode->i_size && i < offset;) {
+			for (i = 0; i < extra_size && i < offset;) {
+				/*
+				 * "." is with offset 0 and
+				 * ".." is dotdot_offset.
+				 */
 				if (!i) {
-					/* skip "." and ".." if needed. */
-					i += EXT4_INLINE_DOTDOT_SIZE;
+					i = dotdot_offset;
+					continue;
+				} else if (i == dotdot_offset) {
+					i = dotdot_size;
 					continue;
 				}
+				/* for other entry, the real offset in
+				 * the buf has to be tuned accordingly.
+				 */
 				de = (struct ext4_dir_entry_2 *)
-					(dir_buf + i);
+					(dir_buf + i - extra_offset);
 				/* It's too expensive to do a full
 				 * dirent test each time round this
 				 * loop, but we do have to test at
@@ -1463,43 +1493,47 @@  revalidate:
 				 * failure will be detected in the
 				 * dirent test below. */
 				if (ext4_rec_len_from_disk(de->rec_len,
-					inline_size) < EXT4_DIR_REC_LEN(1))
+					extra_size) < EXT4_DIR_REC_LEN(1))
 					break;
 				i += ext4_rec_len_from_disk(de->rec_len,
-							    inline_size);
+							    extra_size);
 			}
 			offset = i;
 			filp->f_pos = offset;
 			filp->f_version = inode->i_version;
 		}
 
-		while (!error && filp->f_pos < inode->i_size) {
+		while (!error && filp->f_pos < extra_size) {
 			if (filp->f_pos == 0) {
 				error = filldir(dirent, ".", 1, 0, inode->i_ino,
 						DT_DIR);
 				if (error)
 					break;
 				stored++;
+				filp->f_pos = dotdot_offset;
+				continue;
+			}
 
-				error = filldir(dirent, "..", 2, 0, parent_ino,
-						DT_DIR);
+			if (filp->f_pos == dotdot_offset) {
+				error = filldir(dirent, "..", 2,
+						dotdot_offset,
+						parent_ino, DT_DIR);
 				if (error)
 					break;
 				stored++;
 
-				filp->f_pos = offset = EXT4_INLINE_DOTDOT_SIZE;
+				filp->f_pos = dotdot_size;
 				continue;
 			}
 
-			de = (struct ext4_dir_entry_2 *)(dir_buf + offset);
+			de = (struct ext4_dir_entry_2 *)
+				(dir_buf + filp->f_pos - extra_offset);
 			if (ext4_check_dir_entry(inode, filp, de,
 						 iloc.bh, dir_buf,
-						 inline_size, offset)) {
+						 extra_size, filp->f_pos)) {
 				ret = stored;
 				goto out;
 			}
-			offset += ext4_rec_len_from_disk(de->rec_len,
-							 inline_size);
 			if (le32_to_cpu(de->inode)) {
 				/* We might block in the next section
 				 * if the data destination is
@@ -1522,9 +1556,8 @@  revalidate:
 				stored++;
 			}
 			filp->f_pos += ext4_rec_len_from_disk(de->rec_len,
-							      inline_size);
+							      extra_size);
 		}
-		offset = 0;
 	}
 out:
 	kfree(dir_buf);