linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sergei Antonov <saproj@gmail.com>
To: linux-fsdevel@vger.kernel.org
Cc: Anton Altaparmakov <aia21@cam.ac.uk>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Christoph Hellwig <hch@infradead.org>,
	Vyacheslav Dubeyko <slava@dubeyko.com>,
	Hin-Tak Leung <htl10@users.sourceforge.net>,
	Kyle Laracey <kalaracey@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Sergei Antonov <saproj@gmail.com>
Subject: [PATCH] hfsplus: fix "unused node is not erased" error
Date: Tue, 20 May 2014 19:44:23 +0200	[thread overview]
Message-ID: <1400607863-14633-1-git-send-email-saproj@gmail.com> (raw)

Zero newly allocated extents in the catalog tree if volume attributes tell
us to. Not doing so we risk getting the "unused node is not erased" error.
See kHFSUnusedNodeFix flag in Apple's source code for reference.

There was a previous commit clearing the node when it is freed:
  commit 899bed05e9f6bbb21776f9ebd88f5631987f987a
  Author: Vyacheslav Dubeyko <slava@dubeyko.com>
  Date:   Wed Feb 27 17:03:06 2013 -0800
  hfsplus: fix issue with unzeroed unused b-tree nodes
It did not handle newly allocated extents (this patch fixes it). And it zeroed
nodes in all trees unconditionally which is an overkill. This patch adds a
condition and also switches to 'tree->node_size' as a simpler method of getting
the length to zero.

Cc: Anton Altaparmakov <aia21@cam.ac.uk>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Vyacheslav Dubeyko <slava@dubeyko.com>
Cc: Hin-Tak Leung <htl10@users.sourceforge.net>
Cc: Kyle Laracey <kalaracey@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Sergei Antonov <saproj@gmail.com>
---
 fs/hfsplus/bnode.c       | 17 +++++++++++++++--
 fs/hfsplus/btree.c       |  2 +-
 fs/hfsplus/extents.c     | 10 ++++++++--
 fs/hfsplus/hfsplus_fs.h  |  3 ++-
 fs/hfsplus/hfsplus_raw.h |  1 +
 fs/hfsplus/xattr.c       |  2 +-
 6 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c
index 11c8602..15a844f 100644
--- a/fs/hfsplus/bnode.c
+++ b/fs/hfsplus/bnode.c
@@ -648,8 +648,8 @@ void hfs_bnode_put(struct hfs_bnode *node)
 		if (test_bit(HFS_BNODE_DELETED, &node->flags)) {
 			hfs_bnode_unhash(node);
 			spin_unlock(&tree->hash_lock);
-			hfs_bnode_clear(node, 0,
-				PAGE_CACHE_SIZE * tree->pages_per_bnode);
+			if (hfs_bnode_need_zeroout(tree))
+				hfs_bnode_clear(node, 0, tree->node_size);
 			hfs_bmap_free(node);
 			hfs_bnode_free(node);
 			return;
@@ -658,3 +658,16 @@ void hfs_bnode_put(struct hfs_bnode *node)
 	}
 }
 
+/*
+ * Unused nodes have to be zeroed if this is the catalog tree and
+ * a corresponding flag in the volume header is set.
+ */
+bool hfs_bnode_need_zeroout(struct hfs_btree *tree)
+{
+	struct super_block *sb = tree->inode->i_sb;
+	struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
+	const u32 volume_attr = be32_to_cpu(sbi->s_vhdr->attributes);
+
+	return tree->cnid == HFSPLUS_CAT_CNID &&
+		volume_attr & HFSPLUS_VOL_UNUSED_NODE_FIX;
+}
diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c
index 0fcec8b..3345c75 100644
--- a/fs/hfsplus/btree.c
+++ b/fs/hfsplus/btree.c
@@ -358,7 +358,7 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
 		u32 count;
 		int res;
 
-		res = hfsplus_file_extend(inode);
+		res = hfsplus_file_extend(inode, hfs_bnode_need_zeroout(tree));
 		if (res)
 			return ERR_PTR(res);
 		hip->phys_size = inode->i_size =
diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c
index a7aafb35..a09fcb6 100644
--- a/fs/hfsplus/extents.c
+++ b/fs/hfsplus/extents.c
@@ -235,7 +235,7 @@ int hfsplus_get_block(struct inode *inode, sector_t iblock,
 		if (iblock > hip->fs_blocks || !create)
 			return -EIO;
 		if (ablock >= hip->alloc_blocks) {
-			res = hfsplus_file_extend(inode);
+			res = hfsplus_file_extend(inode, false);
 			if (res)
 				return res;
 		}
@@ -425,7 +425,7 @@ int hfsplus_free_fork(struct super_block *sb, u32 cnid,
 	return res;
 }
 
-int hfsplus_file_extend(struct inode *inode)
+int hfsplus_file_extend(struct inode *inode, bool zeroout)
 {
 	struct super_block *sb = inode->i_sb;
 	struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
@@ -463,6 +463,12 @@ int hfsplus_file_extend(struct inode *inode)
 		}
 	}
 
+	if (zeroout) {
+		res = sb_issue_zeroout(sb, start, len, GFP_NOFS);
+		if (res)
+			goto out;
+	}
+
 	hfs_dbg(EXTENT, "extend %lu: %u,%u\n", inode->i_ino, start, len);
 
 	if (hip->alloc_blocks <= hip->first_blocks) {
diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index 83dc292..3c872b2 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -417,6 +417,7 @@ void hfs_bnode_free(struct hfs_bnode *);
 struct hfs_bnode *hfs_bnode_create(struct hfs_btree *, u32);
 void hfs_bnode_get(struct hfs_bnode *);
 void hfs_bnode_put(struct hfs_bnode *);
+bool hfs_bnode_need_zeroout(struct hfs_btree *);
 
 /* brec.c */
 u16 hfs_brec_lenoff(struct hfs_bnode *, u16, u16 *);
@@ -463,7 +464,7 @@ int hfsplus_ext_write_extent(struct inode *);
 int hfsplus_get_block(struct inode *, sector_t, struct buffer_head *, int);
 int hfsplus_free_fork(struct super_block *, u32,
 		struct hfsplus_fork_raw *, int);
-int hfsplus_file_extend(struct inode *);
+int hfsplus_file_extend(struct inode *, bool zeroout);
 void hfsplus_file_truncate(struct inode *);
 
 /* inode.c */
diff --git a/fs/hfsplus/hfsplus_raw.h b/fs/hfsplus/hfsplus_raw.h
index 5a12682..8298d09 100644
--- a/fs/hfsplus/hfsplus_raw.h
+++ b/fs/hfsplus/hfsplus_raw.h
@@ -144,6 +144,7 @@ struct hfsplus_vh {
 #define HFSPLUS_VOL_NODEID_REUSED	(1 << 12)
 #define HFSPLUS_VOL_JOURNALED		(1 << 13)
 #define HFSPLUS_VOL_SOFTLOCK		(1 << 15)
+#define HFSPLUS_VOL_UNUSED_NODE_FIX	(1 << 31)
 
 /* HFS+ BTree node descriptor */
 struct hfs_bnode_desc {
diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c
index 4e27edc..1ed05d0 100644
--- a/fs/hfsplus/xattr.c
+++ b/fs/hfsplus/xattr.c
@@ -195,7 +195,7 @@ check_attr_tree_state_again:
 	}
 
 	while (hip->alloc_blocks < hip->clump_blocks) {
-		err = hfsplus_file_extend(attr_file);
+		err = hfsplus_file_extend(attr_file, false);
 		if (unlikely(err)) {
 			pr_err("failed to extend attributes file\n");
 			goto end_attr_file_creation;
-- 
1.9.0


             reply	other threads:[~2014-05-20 17:44 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-20 17:44 Sergei Antonov [this message]
2014-05-21 16:40 ` [PATCH] hfsplus: fix "unused node is not erased" error Vyacheslav Dubeyko
2014-05-21 18:15   ` Sergei Antonov
2014-05-22 13:07     ` Vyacheslav Dubeyko
2014-05-22 13:18       ` Sergei Antonov
2014-05-23  6:21         ` Vyacheslav Dubeyko
2014-05-23 10:54           ` Sergei Antonov
  -- strict thread matches above, loose matches on Subject: below --
2014-05-21 20:12 Hin-Tak Leung
2014-05-22  6:29 ` Vyacheslav Dubeyko
2014-05-22  7:07   ` Andrew Morton
2014-05-22 12:48     ` Sergei Antonov
2014-05-22 13:13       ` Vyacheslav Dubeyko
2014-05-22 13:25         ` Sergei Antonov
2014-05-22 14:55           ` Vyacheslav Dubeyko
2014-05-22 17:35             ` Sergei Antonov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1400607863-14633-1-git-send-email-saproj@gmail.com \
    --to=saproj@gmail.com \
    --cc=aia21@cam.ac.uk \
    --cc=akpm@linux-foundation.org \
    --cc=hch@infradead.org \
    --cc=htl10@users.sourceforge.net \
    --cc=kalaracey@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=slava@dubeyko.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).