The exfat entry validator checks the type and ordering of benign secondary
entries, but does not validate Vendor Allocation entry fields. In addition,
exfat_find() reads only the first two entries, allowing malformed trailing
Vendor Allocation entries to bypass validation.

Validate the allocation flags, VendorGuid, FirstCluster, DataLength, and
NoFatChain extent. Read the complete entry set during lookup so malformed
Vendor Allocation entries are rejected.

Signed-off-by: Yang Wen <anmuxixixi@gmail.com>
---
 fs/exfat/dir.c   | 48 +++++++++++++++++++++++++++++++++++++++++++++---
 fs/exfat/namei.c |  3 ++-
 2 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index fe73b1380c5d..951261bd8e55 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -678,11 +678,53 @@ enum exfat_validate_dentry_mode {
 	ES_MODE_GET_BENIGN_SEC_ENTRY,
 };
 
-static bool exfat_validate_entry(unsigned int type,
-		enum exfat_validate_dentry_mode *mode)
+static bool exfat_validate_vendor_alloc(struct super_block *sb,
+		struct exfat_dentry *ep)
+{
+	struct exfat_sb_info *sbi = EXFAT_SB(sb);
+	u8 flags = ep->dentry.vendor_alloc.flags;
+	u32 start_clu = le32_to_cpu(ep->dentry.vendor_alloc.start_clu);
+	u64 size = le64_to_cpu(ep->dentry.vendor_alloc.size);
+	u64 max_size = EXFAT_CLU_TO_B((u64)EXFAT_DATA_CLUSTER_COUNT(sbi), sbi);
+	u64 num_clusters;
+
+	/* AllocationPossible is required for Vendor Allocation entries. */
+	if (!(flags & ALLOC_POSSIBLE))
+		return false;
+
+	/* The null GUID does not identify a valid vendor allocation. */
+	if (!memchr_inv(ep->dentry.vendor_alloc.vendor_guid, 0,
+			sizeof(ep->dentry.vendor_alloc.vendor_guid)))
+		return false;
+
+	if (!start_clu)
+		return !size && !(flags & (ALLOC_NO_FAT_CHAIN ^ ALLOC_FAT_CHAIN));
+
+	if (!is_valid_cluster(sbi, start_clu) || size > max_size)
+		return false;
+
+	if ((flags & ALLOC_NO_FAT_CHAIN) == ALLOC_NO_FAT_CHAIN) {
+		if (!size)
+			return false;
+
+		num_clusters = DIV_ROUND_UP_ULL(size, sbi->cluster_size);
+		if (num_clusters > sbi->num_clusters - start_clu)
+			return false;
+	}
+
+	return true;
+}
+
+static bool exfat_validate_entry(struct super_block *sb,
+		struct exfat_dentry *ep, enum exfat_validate_dentry_mode *mode)
 {
+	unsigned int type = exfat_get_entry_type(ep);
+
 	if (type == TYPE_UNUSED || type == TYPE_DELETED)
 		return false;
+	if (type == TYPE_VENDOR_ALLOC &&
+	    !exfat_validate_vendor_alloc(sb, ep))
+		return false;
 
 	switch (*mode) {
 	case ES_MODE_GET_FILE_ENTRY:
@@ -836,7 +878,7 @@ int exfat_get_dentry_set(struct exfat_entry_set_cache *es,
 	/* validate cached dentries */
 	for (i = ES_IDX_STREAM; i < es->num_entries; i++) {
 		ep = exfat_get_dentry_cached(es, i);
-		if (!exfat_validate_entry(exfat_get_entry_type(ep), &mode))
+		if (!exfat_validate_entry(sb, ep, &mode))
 			goto put_es;
 	}
 	return 0;
diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index a4dc83b5949c..80f60e80786e 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -645,7 +645,8 @@ static int exfat_find(struct inode *dir, const struct qstr *qname,
 	info->entry = dentry;
 	info->num_subdirs = 0;
 
-	if (exfat_get_dentry_set(&es, sb, &cdir, dentry, ES_2_ENTRIES))
+	/* Validate the complete set, including recognized benign entries. */
+	if (exfat_get_dentry_set(&es, sb, &cdir, dentry, ES_ALL_ENTRIES))
 		return -EIO;
 	ep = exfat_get_dentry_cached(&es, ES_IDX_FILE);
 	ep2 = exfat_get_dentry_cached(&es, ES_IDX_STREAM);
-- 
2.34.1

Hi Yang,

> @@ -678,11 +678,53 @@ enum exfat_validate_dentry_mode {
>         ES_MODE_GET_BENIGN_SEC_ENTRY,
>  };
>
> -static bool exfat_validate_entry(unsigned int type,
> -               enum exfat_validate_dentry_mode *mode)
> +static bool exfat_validate_vendor_alloc(struct super_block *sb,
> +               struct exfat_dentry *ep)
> +{
> +       struct exfat_sb_info *sbi = EXFAT_SB(sb);
> +       u8 flags = ep->dentry.vendor_alloc.flags;
> +       u32 start_clu = le32_to_cpu(ep->dentry.vendor_alloc.start_clu);
> +       u64 size = le64_to_cpu(ep->dentry.vendor_alloc.size);
> +       u64 max_size = EXFAT_CLU_TO_B((u64)EXFAT_DATA_CLUSTER_COUNT(sbi), sbi);
Please check the following build error:

fs/exfat/dir.c: In function ‘exfat_validate_vendor_alloc’:
fs/exfat/dir.c:688:24: error: implicit declaration of function
‘EXFAT_CLU_TO_B’; did you mean ‘EXFAT_ACLTAB’?
[-Werror=implicit-function-declaration]
  688 |         u64 max_size =
EXFAT_CLU_TO_B((u64)EXFAT_DATA_CLUSTER_COUNT(sbi), sbi);

Thanks.

Hi,

Thanks for the review.

EXFAT_CLU_TO_B() was replaced by exfat_cluster_to_bytes() in the
current tree. I missed this API change when preparing the patch.

I will send a v2 shortly.

Thanks.

Hi,

A gentle ping on this patch.

When you have time, I’d appreciate any review or feedback.

Thanks!