[PATCH] ntfs: validate usa_ofs before preserving the update sequence number

[PATCH] ntfs: validate usa_ofs before preserving the update sequence number

[Dennis Tighe]

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 <dennis.tighe@gmail.com>
---

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.
+         */
+        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

Re: [PATCH] ntfs: validate usa_ofs before preserving the update sequence number

[Hyunchul Lee]

2026년 8월 18일 (화) 오후 3:05, Dennis Tighe <dennis.tighe@gmail.com>님이 작성:
>
> 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 <dennis.tighe@gmail.com>
> ---
>
> 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.

> +        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