[PATCH 01/12] xfsprogs: some things aren't all that special

[PATCH 01/12] xfsprogs: some things aren't all that special

[Alex Elder]

Move the check for short file names out of is_special_dirent() and
into generate_obfuscated_name().  That way the check is more
directly associated with the algorithm that requires it.

Similarly, move the check for inode == 0, since that case has to do
with storing extended attributes (not files) in the name table.

As a result, is_special_dirent() is really only focused on whether a
given file is in the lost+found directory.

Rename is_special_dirent() to reflect its more specific purpose.

And use a cast to elminate a compile warning in calls to
libxfs_da_hashname().

Signed-off-by: Alex Elder <aelder@sgi.com>

---
 db/metadump.c |   51 +++++++++++++++++++++++++++++----------------------
 1 file changed, 29 insertions(+), 22 deletions(-)

Index: b/db/metadump.c
===================================================================
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2007 Silicon Graphics, Inc.
+ * Copyright (c) 2007,2010 SGI
  * All Rights Reserved.
  *
  * This program is free software; you can redistribute it and/or
@@ -377,8 +377,15 @@ random_filename_char(void)
 	return c;
 }
 
+/*
+ * We won't obfuscate "lost+found" nor any inodes within it.
+ *
+ * Record the "lost+found" directory's inode number when it's found.
+ * While processing that directory, any file whose name matches
+ * its inode number is left as-is (no obfuscation).
+ */
 static int
-is_special_dirent(
+in_lost_found(
 	xfs_ino_t		ino,
 	int			namelen,
 	uchar_t			*name)
@@ -387,22 +394,8 @@ is_special_dirent(
 	char			s[32];
 	int			slen;
 
-	/*
-	 * due to the XFS name hashing algorithm, we cannot obfuscate
-	 * names with 4 chars or less.
-	 */
-	if (namelen <= 4)
-		return 1;
-
-	if (ino == 0)
-		return 0;
-
-	/*
-	 * don't obfuscate lost+found nor any inodes within lost+found with
-	 * the inode number
-	 */
-	if (cur_ino == mp->m_sb.sb_rootino && namelen == 10 &&
-			memcmp(name, "lost+found", 10) == 0) {
+	if (!orphanage_ino && cur_ino == mp->m_sb.sb_rootino &&
+			namelen == 10 && !memcmp(name, "lost+found", 10)) {
 		orphanage_ino = ino;
 		return 1;
 	}
@@ -410,7 +403,8 @@ is_special_dirent(
 		return 0;
 
 	slen = sprintf(s, "%lld", (long long)ino);
-	return (slen == namelen && memcmp(name, s, namelen) == 0);
+
+	return slen == namelen && !memcmp(name, s, namelen);
 }
 
 static void
@@ -426,10 +420,23 @@ generate_obfuscated_name(
 	xfs_dahash_t		newhash;
 	uchar_t			newname[NAME_MAX];
 
-	if (is_special_dirent(ino, namelen, name))
+	/*
+	 * Our obfuscation algorithm requires at least 5-character
+	 * names, so don't bother if the name is too short.
+	 */
+	if (namelen < 5)
+	    	return;
+
+	/*
+	 * We don't obfuscate "lost+found" or any of the files
+	 * therein.  When the name table is used for extended
+	 * attributes, the inode number provided is 0, in which
+	 * case we don't need to make this check.
+	 */
+	if (ino && in_lost_found(ino, namelen, name))
 		return;
 
-	hash = libxfs_da_hashname(name, namelen);
+	hash = libxfs_da_hashname((char *) name, namelen);
 
 	/* create a random name with the same hash value */
 
@@ -468,7 +475,7 @@ generate_obfuscated_name(
 			break;
 		}
 
-		ASSERT(libxfs_da_hashname(newname, namelen) == hash);
+		ASSERT(libxfs_da_hashname((char *) newname, namelen) == hash);
 
 		for (p = nametable[hash % NAME_TABLE_SIZE]; p; p = p->next) {
 			if (p->hash == hash && p->namelen == namelen &&


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 01/12] xfsprogs: some things aren't all that special

[Christoph Hellwig]

On Thu, Dec 30, 2010 at 02:39:59PM -0600, Alex Elder wrote:
> Move the check for short file names out of is_special_dirent() and
> into generate_obfuscated_name().  That way the check is more
> directly associated with the algorithm that requires it.
> 
> Similarly, move the check for inode == 0, since that case has to do
> with storing extended attributes (not files) in the name table.
> 
> As a result, is_special_dirent() is really only focused on whether a
> given file is in the lost+found directory.
> 
> Rename is_special_dirent() to reflect its more specific purpose.
> 
> And use a cast to elminate a compile warning in calls to
> libxfs_da_hashname().

I think Dave's resync of libxfs takes care of that bit.  Either way it
really should be a separate patch.

Also you've stopped overwriting orphanage_ino if it already exists.
I don't think this really matters as we won't have two inodes with the
same name below the root inode.  Either way it should be documented in
the changelog.

Otherwise the change looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 01/12] xfsprogs: some things aren't all that special

[Alex Elder]

On Mon, 2011-01-10 at 15:11 -0500, Christoph Hellwig wrote:
> Also you've stopped overwriting orphanage_ino if it already exists.

Actually, I had noticed I did more (or worse) than
just that.  Once orphanage_ino has been saved, my
change no longer says it should avoid obfuscating
it.  (Or is that what you mean by "overwriting"?)

Anyway, I noticed that problem the other day and
thought I'd fix it if I had a chance to re-post.

Thanks.

					-Alex

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs