Skip to content

Commit 0ca0485

Browse files
gregkhaalexandrovich
authored andcommitted
fs/ntfs3: validate rec->used in journal-replay file record check
check_file_record() validates rec->total against the record size but never validates rec->used. The do_action() journal-replay handlers read rec->used from disk and use it to compute memmove lengths: DeleteAttribute: memmove(attr, ..., used - asize - roff) CreateAttribute: memmove(..., attr, used - roff) change_attr_size: memmove(..., used - PtrOffset(rec, next)) When rec->used is smaller than the offset of a validated attribute, or larger than the record size, these subtractions can underflow allowing us to copy huge amounts of memory in to a 4kb buffer, generally considered a bad idea overall. This requires a corrupted filesystem, which isn't a threat model the kernel really needs to worry about, but checking for such an obvious out-of-bounds value is good to keep things robust, especially on journal replay Fix this up by bounding rec->used correctly. This is much like commit b2bc7c4 ("fs/ntfs3: Fix slab-out-of-bounds read in DeleteIndexEntryRoot") which checked different values in this same switch statement. Cc: Konstantin Komarov <[email protected]> Fixes: b46acd6 ("fs/ntfs3: Add NTFS journal") Cc: stable <[email protected]> Assisted-by: gregkh_clanker_t1000 Signed-off-by: Greg Kroah-Hartman <[email protected]> Signed-off-by: Konstantin Komarov <[email protected]>
1 parent a6cd43f commit 0ca0485

1 file changed

Lines changed: 11 additions & 1 deletion

File tree

fs/ntfs3/fslog.c

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2791,13 +2791,14 @@ static inline bool check_file_record(const struct MFT_REC *rec,
27912791
u16 fn = le16_to_cpu(rec->rhdr.fix_num);
27922792
u16 ao = le16_to_cpu(rec->attr_off);
27932793
u32 rs = sbi->record_size;
2794+
u32 used = le32_to_cpu(rec->used);
27942795

27952796
/* Check the file record header for consistency. */
27962797
if (rec->rhdr.sign != NTFS_FILE_SIGNATURE ||
27972798
fo > (SECTOR_SIZE - ((rs >> SECTOR_SHIFT) + 1) * sizeof(short)) ||
27982799
(fn - 1) * SECTOR_SIZE != rs || ao < MFTRECORD_FIXUP_OFFSET_1 ||
27992800
ao > sbi->record_size - SIZEOF_RESIDENT || !is_rec_inuse(rec) ||
2800-
le32_to_cpu(rec->total) != rs) {
2801+
le32_to_cpu(rec->total) != rs || used > rs || used < ao) {
28012802
return false;
28022803
}
28032804

@@ -2809,6 +2810,15 @@ static inline bool check_file_record(const struct MFT_REC *rec,
28092810
return false;
28102811
}
28112812

2813+
/*
2814+
* The do_action() handlers compute memmove lengths as
2815+
* "rec->used - <offset of validated attr>", which underflows when
2816+
* rec->used is smaller than the attribute walk reached. At this
2817+
* point attr is the ATTR_END marker; rec->used must cover it.
2818+
*/
2819+
if (used < PtrOffset(rec, attr) + sizeof(attr->type))
2820+
return false;
2821+
28122822
return true;
28132823
}
28142824

0 commit comments

Comments
 (0)