[PATCH] hpfs: reject dirents with an invalid length in hpfs_count_dnodes()

[Quchaosheng]

syzbot reports a slab-use-after-free in hpfs_count_dnodes():

  BUG: KASAN: use-after-free in hpfs_count_dnodes+0x854/0xb20 fs/hpfs/dnode.c:773
  Read of size 2 at addr ffff8880471a64d0 by task syz.0.17/5986

  HPFS: de_next_de: de->length = 0
  HPFS: dnode_end_de: dnode->first_free = 7b3184b6

de_next_de() adds the dirent length to the dirent pointer:

	static inline struct hpfs_dirent *de_next_de (struct hpfs_dirent *de)
	{
	  CHKCOND(...);
	  return (void *) de + le16_to_cpu(de->length);
	}

CHKCOND() only prints, it does not abort the operation.  When a corrupted
dnode contains a dirent whose length is zero, de_next_de() therefore
returns the same pointer it was given, and both loops that walk the dirent
chain in hpfs_count_dnodes() spin on one address until they read past the
end of the dnode.

hpfs_map_dnode() does validate dirent lengths, but only while the "check"
mount option is set, and only up to dnode->first_free, so it does not
cover this walk.  The default "check=normal" mount option documents that
"it should not crash", so the walk must be safe on its own.

Reject a dirent with an invalid length before using it to advance, in both
loops.  The validity check matches the one in hpfs_map_dnode(): at least
0x20, at most 292 and a multiple of four.

This only changes the read path; de_next_de() itself is left alone because
it is also used by the write paths (hpfs_add_to_dnode(), hpfs_add_dirent()
and delete_empty_dnode()), where silently clamping a length would hide
filesystem corruption.

Reported-by: syzbot+7d1563afac6cb196a444@xxxxxxxxxxxxxxxxxxxxxxxxx
Link: https://syzkaller.appspot.com/bug?extid=7d1563afac6cb196a444
Signed-off-by: Quchaosheng <quchaosheng000406@xxxxxxx>
---
 fs/hpfs/dnode.c   | 22 ++++++++++++++++++++++
 fs/hpfs/hpfs_fn.h | 14 ++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/fs/hpfs/dnode.c b/fs/hpfs/dnode.c
index 8c6aa060f..224be4124 100644
--- a/fs/hpfs/dnode.c
+++ b/fs/hpfs/dnode.c
@@ -767,6 +767,18 @@ void hpfs_count_dnodes(struct super_block *s, dnode_secno dno, int *n_dnodes,
 				ptr, dno, odno);
 			return;
 		}
+		/*
+		 * de_next_de() returns de itself if the dirent length is
+		 * zero, so a corrupted dnode would make this loop spin on
+		 * one address and read past the end of the dnode.  Require
+		 * a sane length before advancing.
+		 */
+		if (!de_length_valid(de)) {
+			hpfs_brelse4(&qbh);
+			hpfs_error(s, "hpfs_count_dnodes: bad dirent length %u in dnode %08x, dno %08x",
+				(unsigned)le16_to_cpu(de->length), dno, ptr);
+			return;
+		}
 		de = de_next_de(de);
 	}
 	next_de:
@@ -779,6 +791,16 @@ void hpfs_count_dnodes(struct super_block *s, dnode_secno dno, int *n_dnodes,
 	process_de:
 	if (!de->first && !de->last && de->directory && n_subdirs) (*n_subdirs)++;
 	if (!de->first && !de->last && n_items) (*n_items)++;
+	/*
+	 * Same problem here: an invalid dirent length would make the
+	 * "next_de" loop below revisit the same address forever.
+	 */
+	if (!de_length_valid(de)) {
+		hpfs_brelse4(&qbh);
+		hpfs_error(s, "hpfs_count_dnodes: bad dirent length %u in dnode %08x",
+			(unsigned)le16_to_cpu(de->length), dno);
+		return;
+	}
 	if ((de = de_next_de(de)) < dnode_end_de(dnode)) goto next_de;
 	ptr = dno;
 	dno = le32_to_cpu(dnode->up);
diff --git a/fs/hpfs/hpfs_fn.h b/fs/hpfs/hpfs_fn.h
index 237c1c23e..95330fe7a 100644
--- a/fs/hpfs/hpfs_fn.h
+++ b/fs/hpfs/hpfs_fn.h
@@ -126,6 +126,20 @@ static inline struct hpfs_dirent *dnode_end_de (struct dnode *dnode)
 
 /* The dir entry after dir entry de */
 
+/*
+ * A valid dirent length is at least 0x20, at most 292 and a multiple of
+ * four, as checked by hpfs_map_dnode().  de_next_de() adds length to the
+ * dirent pointer, so a length of zero would leave the pointer where it is:
+ * a dirent walk using it would then spin on the same address and read past
+ * the end of the dnode.  A length that is not a multiple of four would
+ * misalign the pointer as well.
+ */
+static inline int de_length_valid (struct hpfs_dirent *de)
+{
+  unsigned int len = le16_to_cpu(de->length);
+  return len >= 0x20 && len <= 292 && !(len & 3);
+}
+
 static inline struct hpfs_dirent *de_next_de (struct hpfs_dirent *de)
 {
   CHKCOND(le16_to_cpu(de->length)>=0x20 && le16_to_cpu(de->length)<0x800,("HPFS: de_next_de: de->length = %x\n",(unsigned)le16_to_cpu(de->length)));
-- 
2.43.0