linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: linux-fsdevel@vger.kernel.org
Subject: hfsplus: fix link corruption
Date: Tue, 12 Oct 2010 15:58:03 +0200	[thread overview]
Message-ID: <20101012135803.GF26867@lst.de> (raw)
In-Reply-To: <20101012135634.GA26867@lst.de>

HFS implements hardlink by using indirect catalog entries that refer to a hidden
directly.  The link target is cached in the dev field in the HFS+ specific
inode, which is also used for the device number for device files, and inside
for passing the nlink value of the indirect node from hfsplus_cat_write_inode
to a helper function.  Now if we happen to write out the indirect node while
hfsplus_link is creating the catalog entry we'll get a link pointing to the
linkid of the current nlink value.  This can easily be reproduced by a large
enough loop of local git-clone operations.

Stop abusing the dev field in the HFS+ inode for short term storage by
refactoring the way the permission structure in the catalog entry is
set up, and rename the dev field to linkid to avoid any confusion.

While we're at it also prevent creating hard links to special files, as
the HFS+ dev and linkid share the same space in the on-disk structure.

Signed-off-by: Christoph Hellwig <hch@tuxera.com>

Index: linux-2.6/fs/hfsplus/dir.c
===================================================================
--- linux-2.6.orig/fs/hfsplus/dir.c	2010-10-11 19:35:00.719004014 +0200
+++ linux-2.6/fs/hfsplus/dir.c	2010-10-11 20:01:06.023253839 +0200
@@ -102,7 +102,7 @@ again:
 	if (IS_ERR(inode))
 		return ERR_CAST(inode);
 	if (S_ISREG(inode->i_mode))
-		HFSPLUS_I(inode)->dev = linkid;
+		HFSPLUS_I(inode)->linkid = linkid;
 out:
 	d_add(dentry, inode);
 	return NULL;
@@ -252,6 +252,8 @@ static int hfsplus_link(struct dentry *s
 
 	if (HFSPLUS_IS_RSRC(inode))
 		return -EPERM;
+	if (!S_ISREG(inode->i_mode))
+		return -EPERM;
 
 	mutex_lock(&sbi->vh_mutex);
 	if (inode->i_ino == (u32)(unsigned long)src_dentry->d_fsdata) {
@@ -268,7 +270,7 @@ static int hfsplus_link(struct dentry *s
 			if (res != -EEXIST)
 				goto out;
 		}
-		HFSPLUS_I(inode)->dev = id;
+		HFSPLUS_I(inode)->linkid = id;
 		cnid = sbi->next_cnid++;
 		src_dentry->d_fsdata = (void *)(unsigned long)cnid;
 		res = hfsplus_create_cat(cnid, src_dir, &src_dentry->d_name, inode);
Index: linux-2.6/fs/hfsplus/inode.c
===================================================================
--- linux-2.6.orig/fs/hfsplus/inode.c	2010-10-11 19:35:00.725012325 +0200
+++ linux-2.6/fs/hfsplus/inode.c	2010-10-11 20:01:06.934260195 +0200
@@ -267,7 +267,13 @@ static void hfsplus_set_perms(struct ino
 	perms->mode = cpu_to_be16(inode->i_mode);
 	perms->owner = cpu_to_be32(inode->i_uid);
 	perms->group = cpu_to_be32(inode->i_gid);
-	perms->dev = cpu_to_be32(HFSPLUS_I(inode)->dev);
+
+	if (S_ISREG(inode->i_mode))
+		perms->dev = cpu_to_be32(inode->i_nlink);
+	else if (S_ISBLK(inode->i_mode) || S_ISCHR(inode->i_mode))
+		perms->dev = cpu_to_be32(inode->i_rdev);
+	else
+		perms->dev = 0;
 }
 
 static int hfsplus_file_open(struct inode *inode, struct file *file)
@@ -491,7 +497,7 @@ int hfsplus_cat_read_inode(struct inode
 
 	type = hfs_bnode_read_u16(fd->bnode, fd->entryoffset);
 
-	HFSPLUS_I(inode)->dev = 0;
+	HFSPLUS_I(inode)->linkid = 0;
 	if (type == HFSPLUS_FOLDER) {
 		struct hfsplus_cat_folder *folder = &entry.folder;
 
@@ -595,10 +601,6 @@ int hfsplus_cat_write_inode(struct inode
 		hfs_bnode_read(fd.bnode, &entry, fd.entryoffset,
 					sizeof(struct hfsplus_cat_file));
 		hfsplus_inode_write_fork(inode, &file->data_fork);
-		if (S_ISREG(inode->i_mode))
-			HFSPLUS_I(inode)->dev = inode->i_nlink;
-		if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode))
-			HFSPLUS_I(inode)->dev = kdev_t_to_nr(inode->i_rdev);
 		hfsplus_set_perms(inode, &file->permissions);
 		if ((file->permissions.rootflags | file->permissions.userflags) & HFSPLUS_FLG_IMMUTABLE)
 			file->flags |= cpu_to_be16(HFSPLUS_FILE_LOCKED);
Index: linux-2.6/fs/hfsplus/catalog.c
===================================================================
--- linux-2.6.orig/fs/hfsplus/catalog.c	2010-10-11 19:35:00.741004643 +0200
+++ linux-2.6/fs/hfsplus/catalog.c	2010-10-11 20:01:06.916256284 +0200
@@ -134,7 +134,7 @@ static int hfsplus_cat_build_record(hfsp
 			file->user_info.fdCreator = cpu_to_be32(HFSP_HFSPLUS_CREATOR);
 			file->user_info.fdFlags = cpu_to_be16(0x100);
 			file->create_date = HFSPLUS_I(sbi->hidden_dir)->create_date;
-			file->permissions.dev = cpu_to_be32(HFSPLUS_I(inode)->dev);
+			file->permissions.dev = cpu_to_be32(HFSPLUS_I(inode)->linkid);
 		}
 		return sizeof(*file);
 	}
Index: linux-2.6/fs/hfsplus/hfsplus_fs.h
===================================================================
--- linux-2.6.orig/fs/hfsplus/hfsplus_fs.h	2010-10-11 19:35:00.753011697 +0200
+++ linux-2.6/fs/hfsplus/hfsplus_fs.h	2010-10-11 20:02:12.131299866 +0200
@@ -178,7 +178,11 @@ struct hfsplus_inode_info {
 	 */
 	struct inode *rsrc_inode;
 	__be32 create_date;
-	u32 dev;
+
+	/*
+	 * Protected by sbi->vh_mutex.
+	 */
+	u32 linkid;
 
 	/*
 	 * Protected by i_mutex.
@@ -427,6 +431,4 @@ static inline struct hfsplus_inode_info
 #define hfsp_ut2mt(t)		__hfsp_ut2mt((t).tv_sec)
 #define hfsp_now2mt()		__hfsp_ut2mt(get_seconds())
 
-#define kdev_t_to_nr(x)		(x)
-
 #endif

  parent reply	other threads:[~2010-10-12 13:58 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-12 13:56 hfsplus updates Christoph Hellwig
2010-10-12 13:56 ` hfsplus: fix oops on mount with corrupted btree extent records Christoph Hellwig
2010-10-12 13:57 ` hfs_bnode_find() can fail, resulting in hfs_bnode_split() breakage Christoph Hellwig
2010-10-12 13:57 ` hfsplus: handle more on-disk corruptions without oopsing Christoph Hellwig
2010-10-12 13:57 ` hfsplus: validate btree flags Christoph Hellwig
2010-10-12 13:58 ` Christoph Hellwig [this message]
2010-10-12 13:58 ` hfsplus: remove superflous rootflags field in hfsplus_inode_info Christoph Hellwig
2010-10-12 13:58 ` hfsplus: create correct initial catalog entries for device files Christoph Hellwig
2010-10-12 13:58 ` hfsplus: remove the unused hfsplus_kmap/hfsplus_kunmap helpers Christoph Hellwig

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=20101012135803.GF26867@lst.de \
    --to=hch@lst.de \
    --cc=linux-fsdevel@vger.kernel.org \
    /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).