Revert "staging: erofs: keep corrupted fs from crashing kernel in erofs_namei()"

This reverts commit d4104c5e783f5d053b97268fb92001d785de7dd5.

Turns out it still needs some more work, I merged it to soon :(

Reported-by: Gao Xiang <gaoxiang25@huawei.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

1 files changed, 78 insertions, 89 deletions

diff --git a/drivers/staging/erofs/namei.c b/drivers/staging/erofs/namei.c
index a1300c420e63..5596c52e246d 100644
--- a/drivers/staging/erofs/namei.c
+++ b/drivers/staging/erofs/namei.c
@@ -15,76 +15,74 @@
 
 #include <trace/events/erofs.h>
 
-struct erofs_qstr {
-	const unsigned char *name;
-	const unsigned char *end;
-};
-
-/* based on the end of qn is accurate and it must have the trailing '\0' */
-static inline int dirnamecmp(const struct erofs_qstr *qn,
-			     const struct erofs_qstr *qd,
-			     unsigned int *matched)
+/* based on the value of qn->len is accurate */
+static inline int dirnamecmp(struct qstr *qn,
+	struct qstr *qd, unsigned int *matched)
 {
-	unsigned int i = *matched;
-
-	/*
-	 * on-disk error, let's only BUG_ON in the debugging mode.
-	 * otherwise, it will return 1 to just skip the invalid name
-	 * and go on (in consideration of the lookup performance).
-	 */
-	DBG_BUGON(qd->name > qd->end);
-
-	/* qd could not have trailing '\0' */
-	/* However it is absolutely safe if < qd->end */
-	while (qd->name + i < qd->end && qd->name[i] != '\0') {
-		if (qn->name[i] != qd->name[i]) {
-			*matched = i;
-			return qn->name[i] > qd->name[i] ? 1 : -1;
+	unsigned int i = *matched, len = min(qn->len, qd->len);
+loop:
+	if (unlikely(i >= len)) {
+		*matched = i;
+		if (qn->len < qd->len) {
+			/*
+			 * actually (qn->len == qd->len)
+			 * when qd->name[i] == '\0'
+			 */
+			return qd->name[i] == '\0' ? 0 : -1;
 		}
-		++i;
+		return (qn->len > qd->len);
+	}
+
+	if (qn->name[i] != qd->name[i]) {
+		*matched = i;
+		return qn->name[i] > qd->name[i] ? 1 : -1;
 	}
-	*matched = i;
-	/* See comments in __d_alloc on the terminating NUL character */
-	return qn->name[i] == '\0' ? 0 : 1;
-}
 
-#define nameoff_from_disk(off, sz)	(le16_to_cpu(off) & ((sz) - 1))
+	++i;
+	goto loop;
+}
 
-static struct erofs_dirent *find_target_dirent(struct erofs_qstr *name,
-					       u8 *data,
-					       unsigned int dirblksize,
-					       const int ndirents)
+static struct erofs_dirent *find_target_dirent(
+	struct qstr *name,
+	u8 *data, int maxsize)
 {
-	int head, back;
+	unsigned int ndirents, head, back;
 	unsigned int startprfx, endprfx;
 	struct erofs_dirent *const de = (struct erofs_dirent *)data;
 
+	/* make sure that maxsize is valid */
+	BUG_ON(maxsize < sizeof(struct erofs_dirent));
+
+	ndirents = le16_to_cpu(de->nameoff) / sizeof(*de);
+
+	/* corrupted dir (may be unnecessary...) */
+	BUG_ON(!ndirents);
+
 	head = 0;
 	back = ndirents - 1;
 	startprfx = endprfx = 0;
 
 	while (head <= back) {
-		const int mid = head + (back - head) / 2;
-		const int nameoff = nameoff_from_disk(de[mid].nameoff,
-						      dirblksize);
+		unsigned int mid = head + (back - head) / 2;
+		unsigned int nameoff = le16_to_cpu(de[mid].nameoff);
 		unsigned int matched = min(startprfx, endprfx);
-		struct erofs_qstr dname = {
-			.name = data + nameoff,
-			.end = unlikely(mid >= ndirents - 1) ?
-				data + dirblksize :
-				data + nameoff_from_disk(de[mid + 1].nameoff,
-							 dirblksize)
-		};
+
+		struct qstr dname = QSTR_INIT(data + nameoff,
+			unlikely(mid >= ndirents - 1) ?
+				maxsize - nameoff :
+				le16_to_cpu(de[mid + 1].nameoff) - nameoff);
 
 		/* string comparison without already matched prefix */
 		int ret = dirnamecmp(name, &dname, &matched);
 
-		if (unlikely(!ret)) {
+		if (unlikely(!ret))
 			return de + mid;
-		} else if (ret > 0) {
+		else if (ret > 0) {
 			head = mid + 1;
 			startprfx = matched;
-		} else {
+		} else if (unlikely(mid < 1))	/* fix "mid" overflow */
+			break;
+		else {
 			back = mid - 1;
 			endprfx = matched;
 		}
@@ -93,13 +91,12 @@ static struct erofs_dirent *find_target_dirent(struct erofs_qstr *name,
 	return ERR_PTR(-ENOENT);
 }
 
-static struct page *find_target_block_classic(struct inode *dir,
-					      struct erofs_qstr *name,
-					      int *_diff,
-					      int *_ndirents)
+static struct page *find_target_block_classic(
+	struct inode *dir,
+	struct qstr *name, int *_diff)
 {
 	unsigned int startprfx, endprfx;
-	int head, back;
+	unsigned int head, back;
 	struct address_space *const mapping = dir->i_mapping;
 	struct page *candidate = ERR_PTR(-ENOENT);
 
@@ -108,34 +105,33 @@ static struct page *find_target_block_classic(struct inode *dir,
 	back = inode_datablocks(dir) - 1;
 
 	while (head <= back) {
-		const int mid = head + (back - head) / 2;
+		unsigned int mid = head + (back - head) / 2;
 		struct page *page = read_mapping_page(mapping, mid, NULL);
 
-		if (!IS_ERR(page)) {
-			struct erofs_dirent *de = kmap_atomic(page);
-			const int nameoff = nameoff_from_disk(de->nameoff,
-							      EROFS_BLKSIZ);
-			const int ndirents = nameoff / sizeof(*de);
+		if (IS_ERR(page)) {
+exact_out:
+			if (!IS_ERR(candidate)) /* valid candidate */
+				put_page(candidate);
+			return page;
+		} else {
 			int diff;
-			unsigned int matched;
-			struct erofs_qstr dname;
+			unsigned int ndirents, matched;
+			struct qstr dname;
+			struct erofs_dirent *de = kmap_atomic(page);
+			unsigned int nameoff = le16_to_cpu(de->nameoff);
 
-			if (unlikely(!ndirents)) {
-				DBG_BUGON(1);
-				put_page(page);
-				page = ERR_PTR(-EIO);
-				goto out;
-			}
+			ndirents = nameoff / sizeof(*de);
+
+			/* corrupted dir (should have one entry at least) */
+			BUG_ON(!ndirents || nameoff > PAGE_SIZE);
 
 			matched = min(startprfx, endprfx);
 
 			dname.name = (u8 *)de + nameoff;
-			if (ndirents == 1)
-				dname.end = (u8 *)de + EROFS_BLKSIZ;
-			else
-				dname.end = (u8 *)de +
-					nameoff_from_disk(de[1].nameoff,
-							  EROFS_BLKSIZ);
+			dname.len = ndirents == 1 ?
+				/* since the rest of the last page is 0 */
+				EROFS_BLKSIZ - nameoff
+				: le16_to_cpu(de[1].nameoff) - nameoff;
 
 			/* string comparison without already matched prefix */
 			diff = dirnamecmp(name, &dname, &matched);
@@ -143,7 +139,7 @@ static struct page *find_target_block_classic(struct inode *dir,
 
 			if (unlikely(!diff)) {
 				*_diff = 0;
-				goto out;
+				goto exact_out;
 			} else if (diff > 0) {
 				head = mid + 1;
 				startprfx = matched;
@@ -151,42 +147,35 @@ static struct page *find_target_block_classic(struct inode *dir,
 				if (likely(!IS_ERR(candidate)))
 					put_page(candidate);
 				candidate = page;
-				*_ndirents = ndirents;
 			} else {
 				put_page(page);
 
+				if (unlikely(mid < 1))	/* fix "mid" overflow */
+					break;
+
 				back = mid - 1;
 				endprfx = matched;
 			}
-			continue;
 		}
-out:		/* free if the candidate is valid */
-		if (!IS_ERR(candidate))
-			put_page(candidate);
-		return page;
 	}
 	*_diff = 1;
 	return candidate;
 }
 
 int erofs_namei(struct inode *dir,
-		struct qstr *name,
-		erofs_nid_t *nid, unsigned int *d_type)
+	struct qstr *name,
+	erofs_nid_t *nid, unsigned int *d_type)
 {
-	int diff, ndirents;
+	int diff;
 	struct page *page;
 	u8 *data;
 	struct erofs_dirent *de;
-	struct erofs_qstr qn;
 
 	if (unlikely(!dir->i_size))
 		return -ENOENT;
 
-	qn.name = name->name;
-	qn.end = name->name + name->len;
-
 	diff = 1;
-	page = find_target_block_classic(dir, &qn, &diff, &ndirents);
+	page = find_target_block_classic(dir, name, &diff);
 
 	if (unlikely(IS_ERR(page)))
 		return PTR_ERR(page);
@@ -195,7 +184,7 @@ int erofs_namei(struct inode *dir,
 	/* the target page has been mapped */
 	de = likely(diff) ?
 		/* since the rest of the last page is 0 */
-		find_target_dirent(&qn, data, EROFS_BLKSIZ, ndirents) :
+		find_target_dirent(name, data, EROFS_BLKSIZ) :
 		(struct erofs_dirent *)data;
 
 	if (likely(!IS_ERR(de))) {