ntfs3: Add bounds checking to mi_enum_attr()
authorlei lu <llfamsec@gmail.com>
Fri, 23 Aug 2024 13:39:44 +0000 (21:39 +0800)
committerKonstantin Komarov <almaz.alexandrovich@paragon-software.com>
Tue, 3 Sep 2024 13:58:38 +0000 (16:58 +0300)
Added bounds checking to make sure that every attr don't stray beyond
valid memory region.

Signed-off-by: lei lu <llfamsec@gmail.com>
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
fs/ntfs3/record.c

index 6c76503edc200da63711188a24102213e9b33454..2a375247b3c09b4275bf0e0ec77d20a315be2ffe 100644 (file)
@@ -223,28 +223,19 @@ struct ATTRIB *mi_enum_attr(struct mft_inode *mi, struct ATTRIB *attr)
                prev_type = 0;
                attr = Add2Ptr(rec, off);
        } else {
-               /* Check if input attr inside record. */
+               /*
+                * We don't need to check previous attr here. There is
+                * a bounds checking in the previous round.
+                */
                off = PtrOffset(rec, attr);
-               if (off >= used)
-                       return NULL;
 
                asize = le32_to_cpu(attr->size);
-               if (asize < SIZEOF_RESIDENT) {
-                       /* Impossible 'cause we should not return such attribute. */
-                       return NULL;
-               }
-
-               /* Overflow check. */
-               if (off + asize < off)
-                       return NULL;
 
                prev_type = le32_to_cpu(attr->type);
                attr = Add2Ptr(attr, asize);
                off += asize;
        }
 
-       asize = le32_to_cpu(attr->size);
-
        /* Can we use the first field (attr->type). */
        if (off + 8 > used) {
                static_assert(ALIGN(sizeof(enum ATTR_TYPE), 8) == 8);
@@ -265,6 +256,12 @@ struct ATTRIB *mi_enum_attr(struct mft_inode *mi, struct ATTRIB *attr)
        if (t32 < prev_type)
                return NULL;
 
+       asize = le32_to_cpu(attr->size);
+       if (asize < SIZEOF_RESIDENT) {
+               /* Impossible 'cause we should not return such attribute. */
+               return NULL;
+       }
+
        /* Check overflow and boundary. */
        if (off + asize < off || off + asize > used)
                return NULL;