Re: [PATCH] ntfs: validate usa_ofs before preserving the update sequence number
Dennis Tighe <[email protected]>
| Newsgroups | org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On Tue, Aug 18, 2026 at 04:46:12PM +0900, Hyunchul Lee wrote: > 2026년 8월 18일 (화) 오후 3:05, Dennis Tighe <[email protected]>님이 작성: > > > > When ntfs_mft_record_alloc() reuses a free mft record it reads the old > > update sequence number straight from the on-disk record: > > > > usn = *(__le16 *)((u8 *)m + le16_to_cpu(m->usa_ofs)); > > > > Here m points into the raw $MFT page-cache folio, which still holds > > unvalidated, MST-protected bytes: the folio is read by a plain > > iomap_read_folio() and neither post_read_mst_fixup() nor > > ntfs_mft_record_check() has run on it (both work on private copies). > > m->usa_ofs is therefore an untrusted u16, and a corrupted record can put > > it past the end of the record so the two-byte read lands outside the > > folio. Reading such a record while creating a file gives, under KASAN: > > > > BUG: KASAN: use-after-free in ntfs_mft_record_alloc+... > > Read of size 2 at addr ... > > ntfs_mft_record_alloc -> __ntfs_create -> ntfs_create -> path_openat > > > > Only preserve the old update sequence number when usa_ofs is even and in > > range, mirroring the check ntfs_mft_record_check() already applies; > > otherwise leave usn zero, which the existing restore below skips. > > > > Fixes: 495e90fa3348 ("ntfs: update attrib operations") > > Assisted-by: Claude:claude-opus-4-8 > > Signed-off-by: Dennis Tighe <[email protected]> > > --- > > > > Reached by creating a file on a mounted image whose $MFT holds a > > corrupted free record. This seems like a fairly low-impact one, but > > given that > > it fires KASAN, it felt worth submitting a patch for. > > > > The KASAN error that I received on 7.1.7 is listed above. On a RW mount > > this is a > > one-shot per image and only trips KASAN when the next page happens to be > > poisoned. > > > > A reproducer is available if you'd like (just reach out privately). > > > > Found and fixed with AI assistance. > > > > fs/ntfs/mft.c | 12 +++++++++++- > > 1 file changed, 11 insertions(+), 1 deletion(-) > > > > diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c > > index fd20d7a..271a265 100644 > > --- a/fs/ntfs/mft.c > > +++ b/fs/ntfs/mft.c > > @@ -2333,7 +2333,17 @@ int ntfs_mft_record_alloc(struct ntfs_volume > > *vol, const int mode, > > * wrong with the previous mft record. > > */ > > seq_no = m->sequence_number; > > - usn = *(__le16 *)((u8 *)m + le16_to_cpu(m->usa_ofs)); > > + /* > > + * The mft record still holds unvalidated, MST-protected on-disk > > + * bytes, so m->usa_ofs is untrusted here. Only preserve the old > > + * update sequence number if that offset is in bounds; otherwise > > + * leave usn zero so it is not restored below. > > + */ > > Could you remove the above comment? > Also, as I mentioned in the previous mail, > I could not apply this patch. > That was a misconfigured email client on my end. Sorry about that! I've fixed both of those and resent. > > + if (!(le16_to_cpu(m->usa_ofs) & 1) && > > + le16_to_cpu(m->usa_ofs) + sizeof(usn) <= vol->mft_record_size) > > + usn = *(__le16 *)((u8 *)m + le16_to_cpu(m->usa_ofs)); > > + else > > + usn = 0; > > err = ntfs_mft_record_layout(vol, bit, m); > > if (unlikely(err)) { > > ntfs_error(vol->sb, "Failed to layout allocated mft record > > 0x%llx.", > > -- > > 2.47.3 > > > > > -- > Thanks, > Hyunchul