All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] hfsplus: read support for directory hardlinks
@ 2011-04-29 10:20 Gustav Munkby
  2011-05-02  8:46 ` Christoph Hellwig
  0 siblings, 1 reply; 12+ messages in thread
From: Gustav Munkby @ 2011-04-29 10:20 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-fsdevel

With OS X 10.5 in general, and Time Machine backups in particular,
Apple added support for directory hardlinks in HFS+. As for file
hardlinks, directory hardlinks are represented by normal files with
special creator and type attributes, storing an pseudo inode number
in the link count, referencing a secret file in a special folder.
This patch extends the existing lookup of file hardlinks to also
support folder hardlinks.

Signed-off-by: Gustav Munkby <grddev@gmail.com>
---
 fs/hfsplus/catalog.c     |    2 +-
 fs/hfsplus/dir.c         |   59 +++++++++++++++++++++++++++++++++++----------
 fs/hfsplus/hfsplus_fs.h  |    1 +
 fs/hfsplus/hfsplus_raw.h |    5 ++++
 fs/hfsplus/super.c       |   55 +++++++++++++++++++++++++++++-------------
 5 files changed, 91 insertions(+), 31 deletions(-)

diff --git a/fs/hfsplus/catalog.c b/fs/hfsplus/catalog.c
index b4ba1b3..e12919a 100644
--- a/fs/hfsplus/catalog.c
+++ b/fs/hfsplus/catalog.c
@@ -109,7 +109,7 @@ static int hfsplus_cat_build_record(hfsplus_cat_entry *entry,
 			folder->attribute_mod_date =
 			folder->access_date = hfsp_now2mt();
 		hfsplus_cat_set_perms(inode, &folder->permissions);
-		if (inode == sbi->hidden_dir)
+		if (inode == sbi->hidden_dir || inode == sbi->alias_dir)
 			/* invisible and namelocked */
 			folder->user_info.frFlags = cpu_to_be16(0x5000);
 		return sizeof(*folder);
diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
index 4df5059..eadff10 100644
--- a/fs/hfsplus/dir.c
+++ b/fs/hfsplus/dir.c
@@ -23,6 +23,33 @@ static inline void hfsplus_instantiate(struct dentry *dentry,
 	d_instantiate(dentry, inode);
 }
 
+static inline u32 hfsplus_hardlink_type(struct super_block *sb,
+					struct hfsplus_cat_file *file)
+{
+	struct inode *hdir = HFSPLUS_SB(sb)->hidden_dir;
+	struct inode *adir = HFSPLUS_SB(sb)->alias_dir;
+	struct inode *root = sb->s_root->d_inode;
+	u32 fdType = be32_to_cpu(file->user_info.fdType);
+	u32 fdCreator = be32_to_cpu(file->user_info.fdCreator);
+	__be32 create_date = file->create_date;
+
+	if (hdir && fdType == HFSP_HARDLINK_TYPE &&
+			fdCreator == HFSP_HFSPLUS_CREATOR &&
+			(create_date == HFSPLUS_I(root)->create_date ||
+				create_date == HFSPLUS_I(hdir)->create_date))
+		return fdType;
+
+	/* Apple's Time Machine creates folder hardlinks
+	 * similarly to normal file hardlinks.
+	 */
+	if (adir && fdType == HFSP_FOLDER_ALIAS_TYPE &&
+			fdCreator == HFSP_MACS_CREATOR &&
+			be32_to_cpu(file->permissions.dev) >= 127)
+		return fdType;
+
+	return 0;
+}
+
 /* Find the entry inside dir named dentry->d_name */
 static struct dentry *hfsplus_lookup(struct inode *dir, struct dentry *dentry,
 				     struct nameidata *nd)
@@ -34,6 +61,7 @@ static struct dentry *hfsplus_lookup(struct inode *dir, struct dentry *dentry,
 	int err;
 	u32 cnid, linkid = 0;
 	u16 type;
+	u32 fdType;
 
 	sb = dir->i_sb;
 
@@ -65,19 +93,20 @@ again:
 			goto fail;
 		}
 		cnid = be32_to_cpu(entry.file.id);
-		if (entry.file.user_info.fdType ==
-				cpu_to_be32(HFSP_HARDLINK_TYPE) &&
-				entry.file.user_info.fdCreator ==
-				cpu_to_be32(HFSP_HFSPLUS_CREATOR) &&
-				(entry.file.create_date ==
-					HFSPLUS_I(HFSPLUS_SB(sb)->hidden_dir)->
-						create_date ||
-				entry.file.create_date ==
-					HFSPLUS_I(sb->s_root->d_inode)->
-						create_date) &&
-				HFSPLUS_SB(sb)->hidden_dir) {
+		fdType = hfsplus_hardlink_type(sb, &entry.file);
+		if (fdType) {
 			struct qstr str;
 			char name[32];
+			struct inode *dir;
+			char *namefmt;
+
+			if (fdType == HFSP_FOLDER_ALIAS_TYPE) {
+				dir = HFSPLUS_SB(sb)->alias_dir;
+				namefmt = "dir_%d";
+			} else {
+				dir = HFSPLUS_SB(sb)->hidden_dir;
+				namefmt = "iNode%d";
+			}
 
 			if (dentry->d_fsdata) {
 				/*
@@ -90,10 +119,10 @@ again:
 				dentry->d_fsdata = (void *)(unsigned long)cnid;
 				linkid =
 					be32_to_cpu(entry.file.permissions.dev);
-				str.len = sprintf(name, "iNode%d", linkid);
+				str.len = sprintf(name, namefmt, linkid);
 				str.name = name;
 				hfsplus_cat_build_key(sb, fd.search_key,
-					HFSPLUS_SB(sb)->hidden_dir->i_ino,
+					dir->i_ino,
 					&str);
 				goto again;
 			}
@@ -195,6 +224,10 @@ static int hfsplus_readdir(struct file *filp, void *dirent, filldir_t filldir)
 			    HFSPLUS_SB(sb)->hidden_dir->i_ino ==
 					be32_to_cpu(entry.folder.id))
 				goto next;
+			if (HFSPLUS_SB(sb)->alias_dir &&
+			    HFSPLUS_SB(sb)->alias_dir->i_ino ==
+					be32_to_cpu(entry.folder.id))
+				goto next;
 			if (filldir(dirent, strbuf, len, filp->f_pos,
 				    be32_to_cpu(entry.folder.id), DT_DIR))
 				break;
diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index d685752..42171ae 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -117,6 +117,7 @@ struct hfsplus_sb_info {
 	struct hfs_btree *attr_tree;
 	struct inode *alloc_file;
 	struct inode *hidden_dir;
+	struct inode *alias_dir;
 	struct nls_table *nls;
 
 	/* Runtime variables */
diff --git a/fs/hfsplus/hfsplus_raw.h b/fs/hfsplus/hfsplus_raw.h
index 927cdd6..c6d8859 100644
--- a/fs/hfsplus/hfsplus_raw.h
+++ b/fs/hfsplus/hfsplus_raw.h
@@ -36,6 +36,11 @@
 #define HFSP_WRAPOFF_EMBEDSIG     0x7C
 #define HFSP_WRAPOFF_EMBEDEXT     0x7E
 
+#define HFSP_ALIASDIR_NAME	".HFS+ Private Directory Data\r"
+
+#define HFSP_FOLDER_ALIAS_TYPE	0x66647270	/* 'fdrp' */
+#define HFSP_MACS_CREATOR	0x4d414353	/* 'MACS' */
+
 #define HFSP_HIDDENDIR_NAME \
 	"\xe2\x90\x80\xe2\x90\x80\xe2\x90\x80\xe2\x90\x80HFS+ Private Data"
 
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index b49b555..1ba6618 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -251,6 +251,7 @@ static void hfsplus_put_super(struct super_block *sb)
 	hfs_btree_close(sbi->ext_tree);
 	iput(sbi->alloc_file);
 	iput(sbi->hidden_dir);
+	iput(sbi->alias_dir);
 	kfree(sbi->s_vhdr);
 	kfree(sbi->s_backup_vhdr);
 	unload_nls(sbi->nls);
@@ -329,12 +330,34 @@ static const struct super_operations hfsplus_sops = {
 	.show_options	= hfsplus_show_options,
 };
 
+static int hfsplus_find_hidden_folder(struct super_block *sb,
+				      struct qstr *name,
+				      struct inode **inode)
+{
+	hfsplus_cat_entry entry;
+	struct hfs_find_data fd;
+
+	hfs_find_init(HFSPLUS_SB(sb)->cat_tree, &fd);
+	hfsplus_cat_build_key(sb, fd.search_key, HFSPLUS_ROOT_CNID, name);
+	if (hfs_brec_read(&fd, &entry, sizeof(entry))) {
+		hfs_find_exit(&fd);
+		return 0;
+	}
+
+	hfs_find_exit(&fd);
+	if (entry.type != cpu_to_be16(HFSPLUS_FOLDER))
+		return -EINVAL;
+
+	*inode = hfsplus_iget(sb, be32_to_cpu(entry.folder.id));
+	if (IS_ERR(*inode))
+		return PTR_ERR(*inode);
+	return 0;
+}
+
 static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
 {
 	struct hfsplus_vh *vhdr;
 	struct hfsplus_sb_info *sbi;
-	hfsplus_cat_entry entry;
-	struct hfs_find_data fd;
 	struct inode *root, *inode;
 	struct qstr str;
 	struct nls_table *nls = NULL;
@@ -447,20 +470,16 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
 
 	str.len = sizeof(HFSP_HIDDENDIR_NAME) - 1;
 	str.name = HFSP_HIDDENDIR_NAME;
-	hfs_find_init(sbi->cat_tree, &fd);
-	hfsplus_cat_build_key(sb, fd.search_key, HFSPLUS_ROOT_CNID, &str);
-	if (!hfs_brec_read(&fd, &entry, sizeof(entry))) {
-		hfs_find_exit(&fd);
-		if (entry.type != cpu_to_be16(HFSPLUS_FOLDER))
-			goto out_put_root;
-		inode = hfsplus_iget(sb, be32_to_cpu(entry.folder.id));
-		if (IS_ERR(inode)) {
-			err = PTR_ERR(inode);
-			goto out_put_root;
-		}
-		sbi->hidden_dir = inode;
-	} else
-		hfs_find_exit(&fd);
+
+	err = hfsplus_find_hidden_folder(sb, &str, &sbi->hidden_dir);
+	if (err)
+		goto out_put_root;
+
+	str.len = sizeof(HFSP_ALIASDIR_NAME) - 1;
+	str.name = HFSP_ALIASDIR_NAME;
+	err = hfsplus_find_hidden_folder(sb, &str, &sbi->alias_dir);
+	if (err)
+		goto out_put_hidden_dir;
 
 	if (!(sb->s_flags & MS_RDONLY)) {
 		/*
@@ -490,13 +509,15 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_root = d_alloc_root(root);
 	if (!sb->s_root) {
 		err = -ENOMEM;
-		goto out_put_hidden_dir;
+		goto out_put_alias_dir;
 	}
 
 	unload_nls(sbi->nls);
 	sbi->nls = nls;
 	return 0;
 
+out_put_alias_dir:
+	iput(sbi->alias_dir);
 out_put_hidden_dir:
 	iput(sbi->hidden_dir);
 out_put_root:
-- 
1.7.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] hfsplus: read support for directory hardlinks
  2011-04-29 10:20 [PATCH] hfsplus: read support for directory hardlinks Gustav Munkby
@ 2011-05-02  8:46 ` Christoph Hellwig
  2011-05-02 12:40   ` Al Viro
  0 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2011-05-02  8:46 UTC (permalink / raw)
  To: Gustav Munkby; +Cc: linux-fsdevel

I've applied it with some minor editing to make the indentation
and variable names match the surrounding code a tad better.

Thanks a lot Gustav!


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] hfsplus: read support for directory hardlinks
  2011-05-02  8:46 ` Christoph Hellwig
@ 2011-05-02 12:40   ` Al Viro
  2011-05-03 14:11     ` Gustav Munkby
  0 siblings, 1 reply; 12+ messages in thread
From: Al Viro @ 2011-05-02 12:40 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Gustav Munkby, linux-fsdevel

On Mon, May 02, 2011 at 10:46:31AM +0200, Christoph Hellwig wrote:
> I've applied it with some minor editing to make the indentation
> and variable names match the surrounding code a tad better.
> 
> Thanks a lot Gustav!

What happens to rename() with that thing?  Specifically, what happens
to loop detection?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] hfsplus: read support for directory hardlinks
  2011-05-02 12:40   ` Al Viro
@ 2011-05-03 14:11     ` Gustav Munkby
  2011-05-03 14:26       ` [PATCH] hfsplus: disable rename of " Gustav Munkby
  2011-05-04  9:30       ` [PATCH] hfsplus: read support for " Christoph Hellwig
  0 siblings, 2 replies; 12+ messages in thread
From: Gustav Munkby @ 2011-05-03 14:11 UTC (permalink / raw)
  To: Al Viro; +Cc: Christoph Hellwig, linux-fsdevel

On 05/02/2011 02:40 PM, Al Viro wrote:
> What happens to rename() with that thing?  Specifically, what happens to
> loop detection?

Good point.

Since my main interest was retrieving contents of a backup, I did not much
consider the impact of write operations. Before the patch, the source files were
exposed as normal files, and the target directories as normal directories, so
one could easily have introduced loops with rename operations before.

In line with the read-only support, I would suggest to conservatively prevent
any rename where the source is a directory hardlink. I have prepared a patch
along those lines, but I cannot test it right now.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] hfsplus: disable rename of directory hardlinks
  2011-05-03 14:11     ` Gustav Munkby
@ 2011-05-03 14:26       ` Gustav Munkby
  2011-05-03 17:10         ` Andreas Dilger
  2011-05-04  9:30       ` [PATCH] hfsplus: read support for " Christoph Hellwig
  1 sibling, 1 reply; 12+ messages in thread
From: Gustav Munkby @ 2011-05-03 14:26 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Al Viro, linux-fsdevel

Signed-off-by: Gustav Munkby <grddev@gmail.com>
---
 fs/hfsplus/dir.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
index 4df5059..3c35296 100644
--- a/fs/hfsplus/dir.c
+++ b/fs/hfsplus/dir.c
@@ -108,7 +108,7 @@ again:
 	inode = hfsplus_iget(dir->i_sb, cnid);
 	if (IS_ERR(inode))
 		return ERR_CAST(inode);
-	if (S_ISREG(inode->i_mode))
+	if (S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode))
 		HFSPLUS_I(inode)->linkid = linkid;
 out:
 	d_add(dentry, inode);
@@ -465,6 +465,13 @@ static int hfsplus_rename(struct inode *old_dir, struct dentry *old_dentry,
 {
 	int res;
 
+	/* Renaming directory hardlinks to could create loops.
+	 * Conservatively prevent any hardlink renaming.
+	 */
+	if (S_ISDIR(old_dentry->d_inode->i_mode) &&
+			HFSPLUS_I(old_dentry->d_inode)->linkid)
+		return -EPERM;
+
 	/* Unlink destination if it already exists */
 	if (new_dentry->d_inode) {
 		if (S_ISDIR(new_dentry->d_inode->i_mode))
-- 
1.7.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] hfsplus: disable rename of directory hardlinks
  2011-05-03 14:26       ` [PATCH] hfsplus: disable rename of " Gustav Munkby
@ 2011-05-03 17:10         ` Andreas Dilger
  2011-05-03 21:29           ` Gustav Munkby
  0 siblings, 1 reply; 12+ messages in thread
From: Andreas Dilger @ 2011-05-03 17:10 UTC (permalink / raw)
  To: Gustav Munkby; +Cc: Christoph Hellwig, Al Viro, linux-fsdevel

On 2011-05-03, at 8:26 AM, Gustav Munkby <grddev@gmail.com> wrote:
> Signed-off-by: Gustav Munkby <grddev@gmail.com>
> ---
> fs/hfsplus/dir.c |    9 ++++++++-
> 1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
> index 4df5059..3c35296 100644
> --- a/fs/hfsplus/dir.c
> +++ b/fs/hfsplus/dir.c
> @@ -108,7 +108,7 @@ again:
>    inode = hfsplus_iget(dir->i_sb, cnid);
>    if (IS_ERR(inode))
>        return ERR_CAST(inode);
> -    if (S_ISREG(inode->i_mode))
> +    if (S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode))
>        HFSPLUS_I(inode)->linkid = linkid;
> out:
>    d_add(dentry, inode);
> @@ -465,6 +465,13 @@ static int hfsplus_rename(struct inode *old_dir, struct dentry *old_dentry,
> {
>    int res;
> 
> +    /* Renaming directory hardlinks to could create loops.
> +     * Conservatively prevent any hardlink renaming.
> +     */

It should also be safe to rename a directory hard link if the source and target parent directories are the same. 

This would allow the case of changing the directory name without moving it to another location.  

> +    if (S_ISDIR(old_dentry->d_inode->i_mode) &&
> +            HFSPLUS_I(old_dentry->d_inode)->linkid)
> +        return -EPERM;
> +
>    /* Unlink destination if it already exists */
>    if (new_dentry->d_inode) {
>        if (S_ISDIR(new_dentry->d_inode->i_mode))
> -- 
> 1.7.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] hfsplus: disable rename of directory hardlinks
  2011-05-03 17:10         ` Andreas Dilger
@ 2011-05-03 21:29           ` Gustav Munkby
  0 siblings, 0 replies; 12+ messages in thread
From: Gustav Munkby @ 2011-05-03 21:29 UTC (permalink / raw)
  To: Andreas Dilger; +Cc: Christoph Hellwig, Al Viro, linux-fsdevel

>> +    /* Renaming directory hardlinks to could create loops.
>> +     * Conservatively prevent any hardlink renaming.
>> +     */
> 
> It should also be safe to rename a directory hard link if the source and target parent directories are the same. 
> 
> This would allow the case of changing the directory name without moving it to another location.  

Theoretically, yes. In fact, it seems the implementation on OS X is even more
liberal by alos allowing moves into other directories as long as the target is
not already beneath another hardlink.

However, for that to make sense, proper support for renaming hardlinks (files as
well as directories) must first be implemented. The way I understand the hfsplus
code both rename and unlink is currently broken for any type of hardlink,
because it operates on the link target rather than the link source. Hardlinks in
HFS+ are represented much like low-level soft links, with one target
file/directory in a private system directory, and several link sources.
Operating on the link target would invalidate all link sources on rename/unlink,
which is clearly not desired.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] hfsplus: read support for directory hardlinks
  2011-05-03 14:11     ` Gustav Munkby
  2011-05-03 14:26       ` [PATCH] hfsplus: disable rename of " Gustav Munkby
@ 2011-05-04  9:30       ` Christoph Hellwig
  2011-05-04 15:04         ` Gustav Munkby
  1 sibling, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2011-05-04  9:30 UTC (permalink / raw)
  To: Gustav Munkby; +Cc: Al Viro, Christoph Hellwig, linux-fsdevel

On Tue, May 03, 2011 at 04:11:31PM +0200, Gustav Munkby wrote:
> On 05/02/2011 02:40 PM, Al Viro wrote:
> > What happens to rename() with that thing?  Specifically, what happens to
> > loop detection?
> 
> Good point.
> 
> Since my main interest was retrieving contents of a backup, I did not much
> consider the impact of write operations. Before the patch, the source files were
> exposed as normal files, and the target directories as normal directories, so
> one could easily have introduced loops with rename operations before.

Given that you have the filesystem around can you clarify where the additional
directory links live?  I was under the impression they are in hidden backup
directories not normally accessible, so there's always just one link in the
normal namespace.

The basic idea would be that we allow any kind of renames in the normal
namespace, but disallow any kind of operation involving the time machine
backups.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] hfsplus: read support for directory hardlinks
  2011-05-04  9:30       ` [PATCH] hfsplus: read support for " Christoph Hellwig
@ 2011-05-04 15:04         ` Gustav Munkby
  2011-05-19 11:09           ` Christoph Hellwig
  0 siblings, 1 reply; 12+ messages in thread
From: Gustav Munkby @ 2011-05-04 15:04 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Al Viro, linux-fsdevel

On 05/04/2011 11:30 AM, Christoph Hellwig wrote:
> On Tue, May 03, 2011 at 04:11:31PM +0200, Gustav Munkby wrote:
>> On 05/02/2011 02:40 PM, Al Viro wrote:
>>> What happens to rename() with that thing?  Specifically, what happens to
>>> loop detection?
>>
>> Good point.
>>
>> Since my main interest was retrieving contents of a backup, I did not much
>> consider the impact of write operations. Before the patch, the source files were
>> exposed as normal files, and the target directories as normal directories, so
>> one could easily have introduced loops with rename operations before.
> 
> Given that you have the filesystem around can you clarify where the additional
> directory links live?  I was under the impression they are in hidden backup
> directories not normally accessible, so there's always just one link in the
> normal namespace.
> 
> The basic idea would be that we allow any kind of renames in the normal
> namespace, but disallow any kind of operation involving the time machine
> backups.
> 

You are correct, the hardlink targets are all inside a hidden private directory.
And then there are hardlink sources both inside and outside this hidden
directory linking to directories inside this directory. While the hidden
directory is not directly accessible to clients, its subdirectories are through
the hardlinks.

Perhaps it was obvious to everyone else, but I just realized that it is not
enough to prevent moving the directory hardlinks themselves, but all directory
renames must be limited (or checked), since the old directory might contain a
nested directory hardlink.Disallowing renames of directories altogether is
clearly not a viable option.

The following three (somewhat overlapping) classes of renames should always be
possible without any risk for introducing loops.

 1) Allow renames where target directory is an ancestor of source directory.
 2) Allow renames that doesn't change the closest hardlink ancestor.
 3) Allow renames into target directories without hardlinked parents.

The first check seems simple enough to implement, but the other two might
require additional locking as far as I understand. Alternatively, they could
both be implemented by adding some additional flags/data to dentry->d_fsdata.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] hfsplus: read support for directory hardlinks
  2011-05-04 15:04         ` Gustav Munkby
@ 2011-05-19 11:09           ` Christoph Hellwig
  2011-05-19 15:57             ` Gustav Munkby
  0 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2011-05-19 11:09 UTC (permalink / raw)
  To: Gustav Munkby; +Cc: Christoph Hellwig, Al Viro, linux-fsdevel

Given that our view of how these links behave was a bit too simplistic
I'll drop the patch from my tree for now.  I'm actually not quite sure
if we can implement the required locking correctly.  It would be
interesting to see what Apple does given that the hfsplus code is
part of the xnu source tarball, but given their general implementaton
quality I'd expect something utterly hacky.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] hfsplus: read support for directory hardlinks
  2011-05-19 11:09           ` Christoph Hellwig
@ 2011-05-19 15:57             ` Gustav Munkby
  2011-05-20 10:30               ` [PATCH v2] hfsplus: readonly " Gustav Munkby
  0 siblings, 1 reply; 12+ messages in thread
From: Gustav Munkby @ 2011-05-19 15:57 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Al Viro, linux-fsdevel

On 05/19/11 13:09, Christoph Hellwig wrote:
> Given that our view of how these links behave was a bit too simplistic
> I'll drop the patch from my tree for now.  I'm actually not quite sure
> if we can implement the required locking correctly.  It would be
> interesting to see what Apple does given that the hfsplus code is
> part of the xnu source tarball, but given their general implementaton
> quality I'd expect something utterly hacky.

I really don't see how the patch makes anything worse. In fact it makes things
slightly better by preventing modifications of the files in the hidden
directory. The raw files are currently exposed and all renames possible with the
patch are equally possible without it.

Moreover, I've done some more tests the last few weeks, and it turns out that
Apple only implements folder hardlinks for journaled HFS+. Since journaled HFS+
is only supported readonly by the hfsplus driver, the write operations are
currently not an issue.

It seems the appropriate read-only patch would ensure that the destination
folder is always hidden, and folder hardlinks are only ever resolved when the
filesystem is mounted read only. This will:
 - prevent creation of folder hardlinks independent of whether
   the filesystem is readonly or not;
 - enable navigation of any folder hardlinks when mounted readonly;
 - ensure all write operations work well for files not beneath a folder
   hardlink, including all files on filesystems without folder hardlinks.

Thoughts?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] hfsplus: readonly support for directory hardlinks
  2011-05-19 15:57             ` Gustav Munkby
@ 2011-05-20 10:30               ` Gustav Munkby
  0 siblings, 0 replies; 12+ messages in thread
From: Gustav Munkby @ 2011-05-20 10:30 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Al Viro, linux-fsdevel, Gustav Munkby

Add support for resolving directory hardlinks as introduced with
Apple OS X 10.5 in general, and Time Machine backups in particular.
Prevent tampering of directory hardlink metadata when mounted writable.
Limit lookup to readonly filesystems to prevent rename from creating
filesystem loops.

Signed-off-by: Gustav Munkby <grddev@gmail.com>
---
 fs/hfsplus/catalog.c     |    2 +-
 fs/hfsplus/dir.c         |   70 ++++++++++++++++++++++++++++++++++++---------
 fs/hfsplus/hfsplus_fs.h  |    1 +
 fs/hfsplus/hfsplus_raw.h |    5 +++
 fs/hfsplus/super.c       |   54 ++++++++++++++++++++++++-----------
 5 files changed, 100 insertions(+), 32 deletions(-)

diff --git a/fs/hfsplus/catalog.c b/fs/hfsplus/catalog.c
index b4ba1b3..e12919a 100644
--- a/fs/hfsplus/catalog.c
+++ b/fs/hfsplus/catalog.c
@@ -109,7 +109,7 @@ static int hfsplus_cat_build_record(hfsplus_cat_entry *entry,
 			folder->attribute_mod_date =
 			folder->access_date = hfsp_now2mt();
 		hfsplus_cat_set_perms(inode, &folder->permissions);
-		if (inode == sbi->hidden_dir)
+		if (inode == sbi->hidden_dir || inode == sbi->alias_dir)
 			/* invisible and namelocked */
 			folder->user_info.frFlags = cpu_to_be16(0x5000);
 		return sizeof(*folder);
diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
index 4df5059..290467f 100644
--- a/fs/hfsplus/dir.c
+++ b/fs/hfsplus/dir.c
@@ -23,6 +23,41 @@ static inline void hfsplus_instantiate(struct dentry *dentry,
 	d_instantiate(dentry, inode);
 }
 
+static inline u32 hfsplus_hardlink_type(struct super_block *sb,
+					struct hfsplus_cat_file *file)
+{
+	struct inode *hdir = HFSPLUS_SB(sb)->hidden_dir;
+	struct inode *adir = HFSPLUS_SB(sb)->alias_dir;
+	struct inode *root = sb->s_root->d_inode;
+	u32 fdtype = be32_to_cpu(file->user_info.fdType);
+	u32 creator = be32_to_cpu(file->user_info.fdCreator);
+	__be32 create_date = file->create_date;
+
+	if (hdir && fdtype == HFSP_HARDLINK_TYPE &&
+			creator == HFSP_HFSPLUS_CREATOR &&
+			(create_date == HFSPLUS_I(root)->create_date ||
+			 create_date == HFSPLUS_I(hdir)->create_date))
+		return fdtype;
+
+	/* Apple's Time Machine creates folder hardlinks similarly to
+	 * normal file hardlinks.
+	 *
+	 * Since write operations rmdir and rename are not yet supported,
+	 * only enable lookup of folder hardlink destinations when hfsplus
+	 * is mounted readonly.
+	 *
+	 * Supporting folder hardlink rmdir would require processing similar
+	 * to unlink for file hardlinks. All folder renames would require
+	 * additional protection to not introduce filesystem loops.  */
+	if (adir && (sb->s_flags & MS_RDONLY) &&
+			fdtype == HFSP_FOLDER_ALIAS_TYPE &&
+			creator == HFSP_MACS_CREATOR &&
+			be32_to_cpu(file->permissions.dev) >= 127)
+		return fdtype;
+
+	return 0;
+}
+
 /* Find the entry inside dir named dentry->d_name */
 static struct dentry *hfsplus_lookup(struct inode *dir, struct dentry *dentry,
 				     struct nameidata *nd)
@@ -34,6 +69,7 @@ static struct dentry *hfsplus_lookup(struct inode *dir, struct dentry *dentry,
 	int err;
 	u32 cnid, linkid = 0;
 	u16 type;
+	u32 fdtype;
 
 	sb = dir->i_sb;
 
@@ -58,26 +94,28 @@ again:
 			goto fail;
 		}
 		cnid = be32_to_cpu(entry.folder.id);
-		dentry->d_fsdata = (void *)(unsigned long)cnid;
+		if (!dentry->d_fsdata)
+			dentry->d_fsdata = (void *)(unsigned long)cnid;
 	} else if (type == HFSPLUS_FILE) {
 		if (fd.entrylength < sizeof(struct hfsplus_cat_file)) {
 			err = -EIO;
 			goto fail;
 		}
 		cnid = be32_to_cpu(entry.file.id);
-		if (entry.file.user_info.fdType ==
-				cpu_to_be32(HFSP_HARDLINK_TYPE) &&
-				entry.file.user_info.fdCreator ==
-				cpu_to_be32(HFSP_HFSPLUS_CREATOR) &&
-				(entry.file.create_date ==
-					HFSPLUS_I(HFSPLUS_SB(sb)->hidden_dir)->
-						create_date ||
-				entry.file.create_date ==
-					HFSPLUS_I(sb->s_root->d_inode)->
-						create_date) &&
-				HFSPLUS_SB(sb)->hidden_dir) {
+		fdtype = hfsplus_hardlink_type(sb, &entry.file);
+		if (fdtype) {
 			struct qstr str;
 			char name[32];
+			struct inode *dir;
+			char *namefmt;
+
+			if (fdtype == HFSP_FOLDER_ALIAS_TYPE) {
+				dir = HFSPLUS_SB(sb)->alias_dir;
+				namefmt = "dir_%d";
+			} else {
+				dir = HFSPLUS_SB(sb)->hidden_dir;
+				namefmt = "iNode%d";
+			}
 
 			if (dentry->d_fsdata) {
 				/*
@@ -90,10 +128,10 @@ again:
 				dentry->d_fsdata = (void *)(unsigned long)cnid;
 				linkid =
 					be32_to_cpu(entry.file.permissions.dev);
-				str.len = sprintf(name, "iNode%d", linkid);
+				str.len = sprintf(name, namefmt, linkid);
 				str.name = name;
 				hfsplus_cat_build_key(sb, fd.search_key,
-					HFSPLUS_SB(sb)->hidden_dir->i_ino,
+					dir->i_ino,
 					&str);
 				goto again;
 			}
@@ -195,6 +233,10 @@ static int hfsplus_readdir(struct file *filp, void *dirent, filldir_t filldir)
 			    HFSPLUS_SB(sb)->hidden_dir->i_ino ==
 					be32_to_cpu(entry.folder.id))
 				goto next;
+			if (HFSPLUS_SB(sb)->alias_dir &&
+			    HFSPLUS_SB(sb)->alias_dir->i_ino ==
+					be32_to_cpu(entry.folder.id))
+				goto next;
 			if (filldir(dirent, strbuf, len, filp->f_pos,
 				    be32_to_cpu(entry.folder.id), DT_DIR))
 				break;
diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index d685752..42171ae 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -117,6 +117,7 @@ struct hfsplus_sb_info {
 	struct hfs_btree *attr_tree;
 	struct inode *alloc_file;
 	struct inode *hidden_dir;
+	struct inode *alias_dir;
 	struct nls_table *nls;
 
 	/* Runtime variables */
diff --git a/fs/hfsplus/hfsplus_raw.h b/fs/hfsplus/hfsplus_raw.h
index 927cdd6..c6d8859 100644
--- a/fs/hfsplus/hfsplus_raw.h
+++ b/fs/hfsplus/hfsplus_raw.h
@@ -36,6 +36,11 @@
 #define HFSP_WRAPOFF_EMBEDSIG     0x7C
 #define HFSP_WRAPOFF_EMBEDEXT     0x7E
 
+#define HFSP_ALIASDIR_NAME	".HFS+ Private Directory Data\r"
+
+#define HFSP_FOLDER_ALIAS_TYPE	0x66647270	/* 'fdrp' */
+#define HFSP_MACS_CREATOR	0x4d414353	/* 'MACS' */
+
 #define HFSP_HIDDENDIR_NAME \
 	"\xe2\x90\x80\xe2\x90\x80\xe2\x90\x80\xe2\x90\x80HFS+ Private Data"
 
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index b49b555..cb20383 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -251,6 +251,7 @@ static void hfsplus_put_super(struct super_block *sb)
 	hfs_btree_close(sbi->ext_tree);
 	iput(sbi->alloc_file);
 	iput(sbi->hidden_dir);
+	iput(sbi->alias_dir);
 	kfree(sbi->s_vhdr);
 	kfree(sbi->s_backup_vhdr);
 	unload_nls(sbi->nls);
@@ -329,12 +330,34 @@ static const struct super_operations hfsplus_sops = {
 	.show_options	= hfsplus_show_options,
 };
 
+static int hfsplus_find_hidden_folder(struct super_block *sb,
+				      struct qstr *name,
+				      struct inode **inode)
+{
+	hfsplus_cat_entry entry;
+	struct hfs_find_data fd;
+
+	hfs_find_init(HFSPLUS_SB(sb)->cat_tree, &fd);
+	hfsplus_cat_build_key(sb, fd.search_key, HFSPLUS_ROOT_CNID, name);
+	if (hfs_brec_read(&fd, &entry, sizeof(entry))) {
+		hfs_find_exit(&fd);
+		return 0;
+	}
+
+	hfs_find_exit(&fd);
+	if (entry.type != cpu_to_be16(HFSPLUS_FOLDER))
+		return -EINVAL;
+
+	*inode = hfsplus_iget(sb, be32_to_cpu(entry.folder.id));
+	if (IS_ERR(*inode))
+		return PTR_ERR(*inode);
+	return 0;
+}
+
 static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
 {
 	struct hfsplus_vh *vhdr;
 	struct hfsplus_sb_info *sbi;
-	hfsplus_cat_entry entry;
-	struct hfs_find_data fd;
 	struct inode *root, *inode;
 	struct qstr str;
 	struct nls_table *nls = NULL;
@@ -447,20 +470,15 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
 
 	str.len = sizeof(HFSP_HIDDENDIR_NAME) - 1;
 	str.name = HFSP_HIDDENDIR_NAME;
-	hfs_find_init(sbi->cat_tree, &fd);
-	hfsplus_cat_build_key(sb, fd.search_key, HFSPLUS_ROOT_CNID, &str);
-	if (!hfs_brec_read(&fd, &entry, sizeof(entry))) {
-		hfs_find_exit(&fd);
-		if (entry.type != cpu_to_be16(HFSPLUS_FOLDER))
-			goto out_put_root;
-		inode = hfsplus_iget(sb, be32_to_cpu(entry.folder.id));
-		if (IS_ERR(inode)) {
-			err = PTR_ERR(inode);
-			goto out_put_root;
-		}
-		sbi->hidden_dir = inode;
-	} else
-		hfs_find_exit(&fd);
+	err = hfsplus_find_hidden_folder(sb, &str, &sbi->hidden_dir);
+	if (err)
+		goto out_put_root;
+
+	str.len = sizeof(HFSP_ALIASDIR_NAME) - 1;
+	str.name = HFSP_ALIASDIR_NAME;
+	err = hfsplus_find_hidden_folder(sb, &str, &sbi->alias_dir);
+	if (err)
+		goto out_put_hidden_dir;
 
 	if (!(sb->s_flags & MS_RDONLY)) {
 		/*
@@ -490,13 +508,15 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_root = d_alloc_root(root);
 	if (!sb->s_root) {
 		err = -ENOMEM;
-		goto out_put_hidden_dir;
+		goto out_put_alias_dir;
 	}
 
 	unload_nls(sbi->nls);
 	sbi->nls = nls;
 	return 0;
 
+out_put_alias_dir:
+	iput(sbi->alias_dir);
 out_put_hidden_dir:
 	iput(sbi->hidden_dir);
 out_put_root:
-- 
1.7.5.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2011-05-20 10:31 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-29 10:20 [PATCH] hfsplus: read support for directory hardlinks Gustav Munkby
2011-05-02  8:46 ` Christoph Hellwig
2011-05-02 12:40   ` Al Viro
2011-05-03 14:11     ` Gustav Munkby
2011-05-03 14:26       ` [PATCH] hfsplus: disable rename of " Gustav Munkby
2011-05-03 17:10         ` Andreas Dilger
2011-05-03 21:29           ` Gustav Munkby
2011-05-04  9:30       ` [PATCH] hfsplus: read support for " Christoph Hellwig
2011-05-04 15:04         ` Gustav Munkby
2011-05-19 11:09           ` Christoph Hellwig
2011-05-19 15:57             ` Gustav Munkby
2011-05-20 10:30               ` [PATCH v2] hfsplus: readonly " Gustav Munkby

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.