[PATCH v2] isofs: validate directory records consistently

[Yichong Chen]

isofs_export_get_parent() assumes that the first two directory records
are valid "." and ".." entries.  A malformed image can provide an
invalid length for the first entry, causing the computed ".." offset to
point outside the received block.

Add a shared directory record validator and use it in NFS get_parent,
readdir and lookup.  This keeps the basic directory record length checks
consistent across all directory users before they consume the name field
or use one record length to find the next entry.

Signed-off-by: Yichong Chen <chenyichong@xxxxxxxxxxxxx>
---
v2:
- Move the directory record validator to namei.c.
- Share the validator with NFS get_parent, readdir and lookup.
- Replace duplicate directory record length checks.

 fs/isofs/dir.c    |  7 ++-----
 fs/isofs/export.c | 10 +++++++++-
 fs/isofs/isofs.h  |  3 +++
 fs/isofs/namei.c  | 28 ++++++++++++++++++++++++----
 4 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/fs/isofs/dir.c b/fs/isofs/dir.c
index cc587cd25162..a96268c9ca41 100644
--- a/fs/isofs/dir.c
+++ b/fs/isofs/dir.c
@@ -149,10 +149,8 @@ static int do_isofs_readdir(struct inode *inode, struct file *file,
 			}
 			de = tmpde;
 		}
-		/* Basic sanity check, whether name doesn't exceed dir entry */
-		if (de_len < sizeof(struct iso_directory_record) ||
-		    de_len < de->name_len[0] +
-					sizeof(struct iso_directory_record)) {
+		if (!isofs_dir_record_valid(de, de == tmpde ? 0 : offset_saved,
+					    de == tmpde ? de_len : bufsize)) {
 			printk(KERN_NOTICE "iso9660: Corrupted directory entry"
 			       " in block %lu of inode %llu\n", block,
 			       inode->i_ino);
@@ -300,4 +298,3 @@ const struct inode_operations isofs_dir_inode_operations =
 	.fileattr_get = isofs_fileattr_get,
 };
 
-
diff --git a/fs/isofs/export.c b/fs/isofs/export.c
index 78f80c1a5c54..4f7fa1d508a1 100644
--- a/fs/isofs/export.c
+++ b/fs/isofs/export.c
@@ -83,13 +83,21 @@ static struct dentry *isofs_export_get_parent(struct dentry *child)
 
 	/* This is the "." entry. */
 	de = (struct iso_directory_record*)bh->b_data;
+	if (!isofs_dir_record_valid(de, 0, child_inode->i_sb->s_blocksize) ||
+	    isonum_711(de->name_len) != 1 || de->name[0] != 0) {
+		printk(KERN_ERR "isofs: Unable to find the \".\" directory for NFS.\n");
+		rv = ERR_PTR(-EACCES);
+		goto out;
+	}
 
 	/* The ".." entry is always the second entry. */
 	parent_offset = (unsigned long)isonum_711(de->length);
 	de = (struct iso_directory_record*)(bh->b_data + parent_offset);
 
 	/* Verify it is in fact the ".." entry. */
-	if ((isonum_711(de->name_len) != 1) || (de->name[0] != 1)) {
+	if (!isofs_dir_record_valid(de, parent_offset,
+				    child_inode->i_sb->s_blocksize) ||
+	    isonum_711(de->name_len) != 1 || de->name[0] != 1) {
 		printk(KERN_ERR "isofs: Unable to find the \"..\" "
 		       "directory for NFS.\n");
 		rv = ERR_PTR(-EACCES);
diff --git a/fs/isofs/isofs.h b/fs/isofs/isofs.h
index 0ec8b24a42ed..dacb9cdae4fd 100644
--- a/fs/isofs/isofs.h
+++ b/fs/isofs/isofs.h
@@ -115,6 +115,9 @@ struct inode;		/* To make gcc happy */
 extern int parse_rock_ridge_inode(struct iso_directory_record *, struct inode *, int relocated);
 extern int get_rock_ridge_filename(struct iso_directory_record *, char *, struct inode *);
 extern int isofs_name_translate(struct iso_directory_record *, char *, struct inode *);
+bool isofs_dir_record_valid(struct iso_directory_record *de,
+			    unsigned long offset,
+			    unsigned long bufsize);
 
 int get_joliet_filename(struct iso_directory_record *, unsigned char *, struct inode *);
 int get_acorn_filename(struct iso_directory_record *, char *, struct inode *);
diff --git a/fs/isofs/namei.c b/fs/isofs/namei.c
index 3ace3d6a55e7..a161b28893d6 100644
--- a/fs/isofs/namei.c
+++ b/fs/isofs/namei.c
@@ -10,6 +10,26 @@
 #include <linux/gfp.h>
 #include "isofs.h"
 
+bool isofs_dir_record_valid(struct iso_directory_record *de,
+			    unsigned long offset,
+			    unsigned long bufsize)
+{
+	unsigned int len;
+	unsigned int name_len;
+	unsigned long min_len = offsetof(struct iso_directory_record, name);
+
+	if (offset > bufsize || bufsize - offset < min_len)
+		return false;
+
+	len = isonum_711(de->length);
+	name_len = isonum_711(de->name_len);
+	if (len < min_len || name_len > len - min_len)
+		return false;
+	if (len > bufsize - offset)
+		return false;
+	return true;
+}
+
 static int
 isofs_cmp(struct dentry *dentry, const char *compare, int dlen)
 {
@@ -88,16 +108,16 @@ isofs_find_entry(struct inode *dir, struct dentry *dentry,
 			de = tmpde;
 		}
 
-		dlen = de->name_len[0];
-		dpnt = de->name;
-		/* Basic sanity check, whether name doesn't exceed dir entry */
-		if (de_len < dlen + sizeof(struct iso_directory_record)) {
+		if (!isofs_dir_record_valid(de, de == tmpde ? 0 : offset_saved,
+					    de == tmpde ? de_len : bufsize)) {
 			printk(KERN_NOTICE "iso9660: Corrupted directory entry"
 			       " in block %lu of inode %llu\n", block,
 			       dir->i_ino);
 			brelse(bh);
 			return 0;
 		}
+		dlen = de->name_len[0];
+		dpnt = de->name;
 
 		if (sbi->s_rock &&
 		    ((i = get_rock_ridge_filename(de, tmpname, dir)))) {
-- 
2.51.0