ROMFS: romfs_lookup() shouldn't be doing a partial name comparison

romfs_lookup() should be using a routine akin to strcmp() on the backing store,
rather than one akin to strncmp().  If it uses the latter, it's liable to match
/bin/shutdown when looking up /bin/sh.

Signed-off-by: David Howells <[email protected]>
Tested-by: Michal Simek <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>

fs/romfs/internal.h

@@ -43,5 +43,5 @@ extern int romfs_dev_read(struct super_block *sb, unsigned long pos,
 			  void *buf, size_t buflen);
 extern ssize_t romfs_dev_strnlen(struct super_block *sb,
 				 unsigned long pos, size_t maxlen);
-extern int romfs_dev_strncmp(struct super_block *sb, unsigned long pos,
-			     const char *str, size_t size);
+extern int romfs_dev_strcmp(struct super_block *sb, unsigned long pos,
+			    const char *str, size_t size);

fs/romfs/storage.c

@@ -67,26 +67,35 @@ static ssize_t romfs_mtd_strnlen(struct super_block *sb,
  * compare a string to one in a romfs image on MTD
  * - return 1 if matched, 0 if differ, -ve if error
  */
-static int romfs_mtd_strncmp(struct super_block *sb, unsigned long pos,
-			     const char *str, size_t size)
+static int romfs_mtd_strcmp(struct super_block *sb, unsigned long pos,
+			    const char *str, size_t size)
 {
-	u_char buf[16];
+	u_char buf[17];
 	size_t len, segment;
 	int ret;
 
-	/* scan the string up to 16 bytes at a time */
+	/* scan the string up to 16 bytes at a time, and attempt to grab the
+	 * trailing NUL whilst we're at it */
+	buf[0] = 0xff;
+
 	while (size > 0) {
-		segment = min_t(size_t, size, 16);
+		segment = min_t(size_t, size + 1, 17);
 		ret = ROMFS_MTD_READ(sb, pos, segment, &len, buf);
 		if (ret < 0)
 			return ret;
+		len--;
 		if (memcmp(buf, str, len) != 0)
 			return 0;
+		buf[0] = buf[len];
 		size -= len;
 		pos += len;
 		str += len;
 	}
 
+	/* check the trailing NUL was */
+	if (buf[0])
+		return 0;
+
 	return 1;
 }
 #endif /* CONFIG_ROMFS_ON_MTD */
@@ -154,28 +163,48 @@ static ssize_t romfs_blk_strnlen(struct super_block *sb,
  * compare a string to one in a romfs image on a block device
  * - return 1 if matched, 0 if differ, -ve if error
  */
-static int romfs_blk_strncmp(struct super_block *sb, unsigned long pos,
-			     const char *str, size_t size)
+static int romfs_blk_strcmp(struct super_block *sb, unsigned long pos,
+			    const char *str, size_t size)
 {
 	struct buffer_head *bh;
 	unsigned long offset;
 	size_t segment;
-	bool x;
+	bool matched, terminated = false;
 
-	/* scan the string up to 16 bytes at a time */
+	/* compare string up to a block at a time */
 	while (size > 0) {
 		offset = pos & (ROMBSIZE - 1);
 		segment = min_t(size_t, size, ROMBSIZE - offset);
 		bh = sb_bread(sb, pos >> ROMBSBITS);
 		if (!bh)
 			return -EIO;
-		x = (memcmp(bh->b_data + offset, str, segment) != 0);
-		brelse(bh);
-		if (x)
-			return 0;
+		matched = (memcmp(bh->b_data + offset, str, segment) == 0);
+
 		size -= segment;
 		pos += segment;
 		str += segment;
+		if (matched && size == 0 && offset + segment < ROMBSIZE) {
+			if (!bh->b_data[offset + segment])
+				terminated = true;
+			else
+				matched = false;
+		}
+		brelse(bh);
+		if (!matched)
+			return 0;
+	}
+
+	if (!terminated) {
+		/* the terminating NUL must be on the first byte of the next
+		 * block */
+		BUG_ON((pos & (ROMBSIZE - 1)) != 0);
+		bh = sb_bread(sb, pos >> ROMBSBITS);
+		if (!bh)
+			return -EIO;
+		matched = !bh->b_data[0];
+		brelse(bh);
+		if (!matched)
+			return 0;
 	}
 
 	return 1;
@@ -234,10 +263,12 @@ ssize_t romfs_dev_strnlen(struct super_block *sb,
 
 /*
  * compare a string to one in romfs
+ * - the string to be compared to, str, may not be NUL-terminated; instead the
+ *   string is of the specified size
  * - return 1 if matched, 0 if differ, -ve if error
  */
-int romfs_dev_strncmp(struct super_block *sb, unsigned long pos,
-		      const char *str, size_t size)
+int romfs_dev_strcmp(struct super_block *sb, unsigned long pos,
+		     const char *str, size_t size)
 {
 	size_t limit;
 
@@ -246,16 +277,16 @@ int romfs_dev_strncmp(struct super_block *sb, unsigned long pos,
 		return -EIO;
 	if (size > ROMFS_MAXFN)
 		return -ENAMETOOLONG;
-	if (size > limit - pos)
+	if (size + 1 > limit - pos)
 		return -EIO;
 
 #ifdef CONFIG_ROMFS_ON_MTD
 	if (sb->s_mtd)
-		return romfs_mtd_strncmp(sb, pos, str, size);
+		return romfs_mtd_strcmp(sb, pos, str, size);
 #endif
 #ifdef CONFIG_ROMFS_ON_BLOCK
 	if (sb->s_bdev)
-		return romfs_blk_strncmp(sb, pos, str, size);
+		return romfs_blk_strcmp(sb, pos, str, size);
 #endif
 	return -EIO;
 }

fs/romfs/super.c

@@ -240,8 +240,8 @@ static struct dentry *romfs_lookup(struct inode *dir, struct dentry *dentry,
 			goto error;
 
 		/* try to match the first 16 bytes of name */
-		ret = romfs_dev_strncmp(dir->i_sb, offset + ROMFH_SIZE, name,
-					len);
+		ret = romfs_dev_strcmp(dir->i_sb, offset + ROMFH_SIZE, name,
+				       len);
 		if (ret < 0)
 			goto error;
 		if (ret == 1)