[PATCH] fs/ntfs3: Fix heap overflow after ALIGN in mi_pack_runs()
Alexandro Calò <[email protected]> Wed, 8 Jul 2026 14:56:46 +0000
| Newsgroups | dev.linux.lists.ntfs3,org.kernel.vger.linux-kernel,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
From 70cdcdc3203f73fe2a77b70aa32d31140290cc99 Mon Sep 17 00:00:00 2001 From: Alexandro Calo <[email protected]> Date: Wed, 8 Jul 2026 16:16:39 +0200 Subject: [PATCH] fs/ntfs3: Fix heap overflow after ALIGN in mi_pack_runs() When run_pack() returns a non-aligned byte count, ALIGN(err, 8) inflates it by 1-7 bytes, and since the destination of the memmove() is computed from that inflated value, the entire attribute tail could be copied 1-7 bytes past where the record buffer ends. The memmove() destination is next + new_run_size - run_size. For it to stay within the record, new_run_size must not exceed run_size + dsize, the space actually available. Since new_run_size =3D ALIGN(err, 8), a fix could be to ensures there is enough room that whatever ALIGN does to err, the result still fits within the available space. Restricting the budget of run_pack() should guarantee that. Something like: u32 avail =3D (run_size + dsize) & ~7u; err =3D run_pack(run, svcn, len, Add2Ptr(attr, run_off), avail, &plen); Now, run_pack() should never return 0. As now, the function returns at least 1. So this is fine. Negative values are caught as errors by if(err<0). The rounded-down avail may be smaller than the existing run size, if avail=3D0 run_pack is called and will write run_buf[0] =3D 0; that will = be OOB, because the buffer size is zero, so imo the caller should avoid calling run_pack() with avail=3D0. After run_pack(), if plen =3D 0, then svcn + plen - 1 becomes svcn - 1. So, after run_pack(), it may be worth ensuring if(!plen). Lastly, I'm not sure if -ENOSPC is the right error code here, but it seems reasonable for a space condition. This heap out-of-bounds write requires a crafted filesystem image, which is not in the kernel threat model, but fixing memory errors would be nice to keep things secure. Signed-off-by: Alexandro Calo <[email protected]> --- fs/ntfs3/record.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/fs/ntfs3/record.c b/fs/ntfs3/record.c index 32bdb034c2a3..2466dba15241 100644 --- a/fs/ntfs3/record.c +++ b/fs/ntfs3/record.c @@ -637,18 +637,32 @@ int mi_pack_runs(struct mft_inode *mi, struct ATTRIB = *attr, u32 run_size =3D asize - run_off; u32 tail =3D used - aoff - asize; u32 dsize =3D sbi->record_size - used; + u32 avail; =20 /* Make a maximum gap in current record. */ memmove(next + dsize, next, tail); =20 + /* Leave room for the 8-byte ALIGN() to avoid OOB write */ + avail =3D (run_size + dsize) & ~7u; + + if (!avail) { + memmove(next, next + dsize, tail); + return -ENOSPC; + } + /* Pack as much as possible. */ - err =3D run_pack(run, svcn, len, Add2Ptr(attr, run_off), run_size + dsize= , + err =3D run_pack(run, svcn, len, Add2Ptr(attr, run_off), avail, &plen); if (err < 0) { memmove(next, next + dsize, tail); return err; } =20 + if (!plen) { + memmove(next, next + dsize, tail); + return -ENOSPC; + } + new_run_size =3D ALIGN(err, 8); =20 memmove(next + new_run_size - run_size, next + dsize, tail); base-commit: 0e35b9b6ec0ffcc5e23cbdec09f5c622ad532b53 -- 2.47.3