Skip to contentSkip to conversation

xfs_repair: do not check symlink component lengths

PATCH In review Eric Sandeen 2 participants 2 messages Dec 5, 2014 – Dec 18, 2014

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 xfs_repair: do not check symlink component lengths

As reported by Andy Grimm,

# ln -s $( python -c 'print "a" * 260' ) /mnt/foo

will succeed on xfs, but then xfs_repair will complain:

component of symlink in inode 131 too long
problem with symbolic link in inode 131
would have cleared inode 131

The kernel checks the total length of the symlink on both read and write, but does not look at component paths.

Looking around the kernel, no other filesystem checks component lengths, nor does the vfs. And as Andy points out, the target could even be on a different filesystem, with different limitations.

And having a "too-long" component doesn't even seem like something likely to stem from disk corruption anyway, so I'm not sure why repair should care.

Therefore I propose removing the component length checks from xfs_repair.

Andy Grimm <[email protected]>

---

repair/dinode.c +1 -26
index 38a6562..73e4b9e 100644
@@ -1333,7 +1333,7 @@ process_symlink(
 	xfs_dinode_t	*dino,
 	blkmap_t 	*blkmap)
 {
-	char			*symlink, *cptr;
+	char			*symlink;
 	char			data[MAXPATHLEN];
 
 	/*
@@ -1380,31 +1380,6 @@ _("found illegal null character in symlink inode %" PRIu64 "\n"),
 		return(1);
 	}
 
-	/*
-	 * check for any component being too long
-	 */
-	if (be64_to_cpu(dino->di_size) >= MAXNAMELEN)  {
-		cptr = strchr(symlink, '/');
-
-		while (cptr != NULL)  {
-			if (cptr - symlink >= MAXNAMELEN)  {
-				do_warn(
-_("component of symlink in inode %" PRIu64 " too long\n"),
-					lino);
-				return(1);
-			}
-			symlink = cptr + 1;
-			cptr = strchr(symlink, '/');
-		}
-
-		if (strlen(symlink) >= MAXNAMELEN)  {
-			do_warn(
-_("component of symlink in inode %" PRIu64 " too long\n"),
-				lino);
-			return(1);
-		}
-	}
-
 	return(0);
 }
 

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Reviewed-by on v1 1/1
#1 lore raw
Quoted 10 lines · Eric Sandeen

As reported by Andy Grimm,

# ln -s $( python -c 'print "a" * 260' ) /mnt/foo

will succeed on xfs, but then xfs_repair will complain:

component of symlink in inode 131 too long
problem with symbolic link in inode 131
would have cleared inode 131

Quoted 7 lines

The kernel checks the total length of the symlink on both read and write, but does not look at component paths.

Looking around the kernel, no other filesystem checks component lengths, nor does the vfs. And as Andy points out, the target could even be on a different filesystem, with different limitations.

Interesting, but we do enforce dentry name limits, yes? At least, I can't create such a component on an XFS fs (haven't dug into where that is enforced).

Quoted 4 lines

And having a "too-long" component doesn't even seem like something likely to stem from disk corruption anyway, so I'm not sure why repair should care.

My guess would be the intent is based on the above, where we can't create such a long component name and thus a component that exceeds the limits must be "invalid."

That said, a broken symlink is generally valid and not the same as corruption, so I think I agree that this is somewhat misplaced functionality...

Quoted 5 lines

Therefore I propose removing the component length checks from xfs_repair.

Andy Grimm <[email protected]>

---

Quoted 47 lines
repair/dinode.c +1 -26
index 38a6562..73e4b9e 100644
@@ -1333,7 +1333,7 @@ process_symlink(
 	xfs_dinode_t	*dino,
 	blkmap_t 	*blkmap)
 {
-	char			*symlink, *cptr;
+	char			*symlink;
 	char			data[MAXPATHLEN];
 
 	/*
@@ -1380,31 +1380,6 @@ _("found illegal null character in symlink inode %" PRIu64 "\n"),
 		return(1);
 	}
 
-	/*
-	 * check for any component being too long
-	 */
-	if (be64_to_cpu(dino->di_size) >= MAXNAMELEN)  {
-		cptr = strchr(symlink, '/');
-
-		while (cptr != NULL)  {
-			if (cptr - symlink >= MAXNAMELEN)  {
-				do_warn(
-_("component of symlink in inode %" PRIu64 " too long\n"),
-					lino);
-				return(1);
-			}
-			symlink = cptr + 1;
-			cptr = strchr(symlink, '/');
-		}
-
-		if (strlen(symlink) >= MAXNAMELEN)  {
-			do_warn(
-_("component of symlink in inode %" PRIu64 " too long\n"),
-				lino);
-			return(1);
-		}
-	}
-
 	return(0);
 }
 
Quoted 4 lines

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs