On 23.08.2024 16:39, lei lu wrote:
Added bounds checking to make sure that every attr don't stray beyond
valid memory region.
Signed-off-by: lei lu <llfamsec@xxxxxxxxx>
---
fs/ntfs3/record.c | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/fs/ntfs3/record.c b/fs/ntfs3/record.c
index 6aa3a9d44df1..973e2a371bc2 100644
--- a/fs/ntfs3/record.c
+++ b/fs/ntfs3/record.c
@@ -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;
Hi Lei Lu,
I saw your patch and have started working on it, including checks and
testing.
Regards, Konstantin
