tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

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 b2bc7c44ed17 ("fs/ntfs3: Fix slab-out-of-bounds
read in DeleteIndexEntryRoot") which checked different values in this
same switch statement.

Cc: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
Fixes: b46acd6a6a62 ("fs/ntfs3: Add NTFS journal")
Cc: stable <stable@kernel.org>
Assisted-by: gregkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>

authored by

Greg Kroah-Hartman and committed by
Konstantin Komarov 0ca0485e a6cd43fe

+11 -1
+11 -1
fs/ntfs3/fslog.c
··· 2791 2791 u16 fn = le16_to_cpu(rec->rhdr.fix_num); 2792 2792 u16 ao = le16_to_cpu(rec->attr_off); 2793 2793 u32 rs = sbi->record_size; 2794 + u32 used = le32_to_cpu(rec->used); 2794 2795 2795 2796 /* Check the file record header for consistency. */ 2796 2797 if (rec->rhdr.sign != NTFS_FILE_SIGNATURE || 2797 2798 fo > (SECTOR_SIZE - ((rs >> SECTOR_SHIFT) + 1) * sizeof(short)) || 2798 2799 (fn - 1) * SECTOR_SIZE != rs || ao < MFTRECORD_FIXUP_OFFSET_1 || 2799 2800 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) { 2801 2802 return false; 2802 2803 } 2803 2804 ··· 2809 2808 continue; 2810 2809 return false; 2811 2810 } 2811 + 2812 + /* 2813 + * The do_action() handlers compute memmove lengths as 2814 + * "rec->used - <offset of validated attr>", which underflows when 2815 + * rec->used is smaller than the attribute walk reached. At this 2816 + * point attr is the ATTR_END marker; rec->used must cover it. 2817 + */ 2818 + if (used < PtrOffset(rec, attr) + sizeof(attr->type)) 2819 + return false; 2812 2820 2813 2821 return true; 2814 2822 }