Skip to content

Commit 8afc24a

Browse files
aalexandrovichgregkh
authored andcommitted
fs/ntfs3: add bounds check to run_get_highest_vcn()
[ Upstream commit bb11485 ] run_get_highest_vcn() parses a packed NTFS mapping-pairs buffer without any length bound, relying solely on a 0x00 terminator to stop. A crafted $LogFile UpdateMappingPairs record whose embedded attribute contains mapping-pairs runs without a terminator causes the function to read past the slab allocation, triggering a KASAN slab-out-of-bounds read on mount. The sibling function run_unpack() received an analogous bounds-check in commit b62567b ("ntfs3: add buffer boundary checks to run_unpack()"), but run_get_highest_vcn() was missed. Take a run_buf_size parameter and reject any run header whose payload would extend past the buffer end, mirroring the pattern used by run_unpack(). The caller in fslog.c passes the remaining attribute bytes after the mapping-pairs offset. KASAN report (on mainline v7.1 merge window HEAD): BUG: KASAN: slab-out-of-bounds in run_get_highest_vcn+0x3c0/0x410 Read of size 1 at addr ffff88800e2d5400 by task mount/72 Call Trace: run_get_highest_vcn+0x3c0/0x410 do_action.isra.0+0x3ba8/0x7b50 log_replay+0x9ddd/0x10200 ntfs_loadlog_and_replay+0x4ad/0x610 ntfs_fill_super+0x214a/0x4540 Fixes: b62567b ("ntfs3: add buffer boundary checks to run_unpack()") Signed-off-by: Jaeyeong Lee <lee@jaeyeong.cc> Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent 6c63a4e commit 8afc24a

3 files changed

Lines changed: 13 additions & 4 deletions

File tree

fs/ntfs3/fslog.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3364,7 +3364,10 @@ static int do_action(struct ntfs_log *log, struct OPEN_ATTR_ENRTY *oe,
33643364
memmove(Add2Ptr(attr, aoff), data, dlen);
33653365

33663366
if (run_get_highest_vcn(le64_to_cpu(attr->nres.svcn),
3367-
attr_run(attr), &t64)) {
3367+
attr_run(attr),
3368+
le32_to_cpu(attr->size) -
3369+
le16_to_cpu(attr->nres.run_off),
3370+
&t64)) {
33683371
goto dirty_vol;
33693372
}
33703373

fs/ntfs3/ntfs_fs.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -816,7 +816,8 @@ int run_unpack_ex(struct runs_tree *run, struct ntfs_sb_info *sbi, CLST ino,
816816
#else
817817
#define run_unpack_ex run_unpack
818818
#endif
819-
int run_get_highest_vcn(CLST vcn, const u8 *run_buf, u64 *highest_vcn);
819+
int run_get_highest_vcn(CLST vcn, const u8 *run_buf, size_t run_buf_size,
820+
u64 *highest_vcn);
820821
int run_clone(const struct runs_tree *run, struct runs_tree *new_run);
821822

822823
/* Globals from super.c */

fs/ntfs3/run.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1155,18 +1155,23 @@ int run_unpack_ex(struct runs_tree *run, struct ntfs_sb_info *sbi, CLST ino,
11551155
* Return the highest vcn from a mapping pairs array
11561156
* it used while replaying log file.
11571157
*/
1158-
int run_get_highest_vcn(CLST vcn, const u8 *run_buf, u64 *highest_vcn)
1158+
int run_get_highest_vcn(CLST vcn, const u8 *run_buf, size_t run_buf_size,
1159+
u64 *highest_vcn)
11591160
{
1161+
const u8 *run_last = run_buf + run_buf_size;
11601162
u64 vcn64 = vcn;
11611163
u8 size_size;
11621164

1163-
while ((size_size = *run_buf & 0xF)) {
1165+
while (run_buf < run_last && (size_size = *run_buf & 0xF)) {
11641166
u8 offset_size = *run_buf++ >> 4;
11651167
u64 len;
11661168

11671169
if (size_size > 8 || offset_size > 8)
11681170
return -EINVAL;
11691171

1172+
if (run_buf + size_size + offset_size > run_last)
1173+
return -EINVAL;
1174+
11701175
len = run_unpack_s64(run_buf, size_size, 0);
11711176
if (!len)
11721177
return -EINVAL;

0 commit comments

Comments
 (0)