1. Patchew
  2. linux
  3. View series

[PATCH] fs/ntfs3: bound NTFS_DE view.data_off in UpdateRecordData{Root,Allocation}

Michael Bommarito posted 1 patch 1 week, 2 days ago
There is a newer version of this series
fs/ntfs3/fslog.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
[PATCH] fs/ntfs3: bound NTFS_DE view.data_off in UpdateRecordData{Root,Allocation}
Posted by Michael Bommarito 1 week, 2 days ago 
In do_action()'s UpdateRecordDataRoot (fslog.c:3489) and
UpdateRecordDataAllocation (fslog.c:3697) cases, the memmove
destination is `Add2Ptr(e, le16_to_cpu(e->view.data_off))`,
where e->view.data_off comes from an on-disk NTFS_DE inside
an INDEX_ROOT or INDEX_BUFFER.  Neither case validates
view.data_off + dlen against e->size; the existing
check_if_index_root / check_if_alloc_index helpers walk the
entry chain and validate the entry's offset, but not its
internal view fields.

The neighbouring read sites (e.g., fs/ntfs3/index.c when
iterating view entries) check view.data_off + view.data_size
<= e->size.  Apply the same bound at the two memmove sites.

Reproduced under UML+KASAN on mainline 8d90b09e6741 via
pr_warn-only probe instrumentation: with view.data_off forced
to 0xFFFC, the memmove writes 32 bytes past the end of the
NTFS_DE.

This is similar in shape to Pavitra Jha's 2026-05-02 patch
"fs/ntfs3: prevent oob in case UpdateRecordDataRoot"
(<20260502105008.21827-1-jhapavitra98@gmail.com>) which
proposes calling ntfs3_bad_de_range(); that helper does not
exist in mainline.  This patch uses inline checks.

Fixes: b46acd6a6a62 ("fs/ntfs3: Add NTFS journal")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 fs/ntfs3/fslog.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
index acfa18b84401e..127860fd2ab50 100644
--- a/fs/ntfs3/fslog.c
+++ b/fs/ntfs3/fslog.c
@@ -3497,6 +3497,18 @@ static int do_action(struct ntfs_log *log, struct OPEN_ATTR_ENRTY *oe,
 
 		e = Add2Ptr(attr, le16_to_cpu(lrh->attr_off));
 
+		/*
+		 * e->view.data_off and dlen come from the on-disk
+		 * INDEX_ROOT entry / LRH.  The neighbouring read sites
+		 * (e.g. fs/ntfs3/index.c) check that
+		 * view.data_off + view.data_size <= e->size; mirror that
+		 * bound here so the memmove cannot reach past the entry.
+		 */
+		if (le16_to_cpu(e->view.data_off) > le16_to_cpu(e->size) ||
+		    le16_to_cpu(e->view.data_off) + dlen >
+			    le16_to_cpu(e->size))
+			goto dirty_vol;
+
 		memmove(Add2Ptr(e, le16_to_cpu(e->view.data_off)), data, dlen);
 
 		mi->dirty = true;
@@ -3689,6 +3701,12 @@ static int do_action(struct ntfs_log *log, struct OPEN_ATTR_ENRTY *oe,
 			goto dirty_vol;
 		}
 
+		/* See UpdateRecordDataRoot for the rationale. */
+		if (le16_to_cpu(e->view.data_off) > le16_to_cpu(e->size) ||
+		    le16_to_cpu(e->view.data_off) + dlen >
+			    le16_to_cpu(e->size))
+			goto dirty_vol;
+
 		memmove(Add2Ptr(e, le16_to_cpu(e->view.data_off)), data, dlen);
 
 		a_dirty = true;
-- 
2.53.0
Re: [PATCH] fs/ntfs3: bound NTFS_DE view.data_off in UpdateRecordData{Root,Allocation}
Posted by Pavitra Jha 1 week, 2 days ago 
Thanks for fixing this. Since you cited my original report as the
basis, would you mind adding:

Reported-by: Pavitra Jha <jhapavitra98@gmail.com>

to the patch tags?

On Fri, 15 May 2026 at 22:04, Michael Bommarito
<michael.bommarito@gmail.com> wrote:
>
> In do_action()'s UpdateRecordDataRoot (fslog.c:3489) and
> UpdateRecordDataAllocation (fslog.c:3697) cases, the memmove
> destination is `Add2Ptr(e, le16_to_cpu(e->view.data_off))`,
> where e->view.data_off comes from an on-disk NTFS_DE inside
> an INDEX_ROOT or INDEX_BUFFER.  Neither case validates
> view.data_off + dlen against e->size; the existing
> check_if_index_root / check_if_alloc_index helpers walk the
> entry chain and validate the entry's offset, but not its
> internal view fields.
>
> The neighbouring read sites (e.g., fs/ntfs3/index.c when
> iterating view entries) check view.data_off + view.data_size
> <= e->size.  Apply the same bound at the two memmove sites.
>
> Reproduced under UML+KASAN on mainline 8d90b09e6741 via
> pr_warn-only probe instrumentation: with view.data_off forced
> to 0xFFFC, the memmove writes 32 bytes past the end of the
> NTFS_DE.
>
> This is similar in shape to Pavitra Jha's 2026-05-02 patch
> "fs/ntfs3: prevent oob in case UpdateRecordDataRoot"
> (<20260502105008.21827-1-jhapavitra98@gmail.com>) which
> proposes calling ntfs3_bad_de_range(); that helper does not
> exist in mainline.  This patch uses inline checks.
>
> Fixes: b46acd6a6a62 ("fs/ntfs3: Add NTFS journal")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
>  fs/ntfs3/fslog.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
> index acfa18b84401e..127860fd2ab50 100644
> --- a/fs/ntfs3/fslog.c
> +++ b/fs/ntfs3/fslog.c
> @@ -3497,6 +3497,18 @@ static int do_action(struct ntfs_log *log, struct OPEN_ATTR_ENRTY *oe,
>
>                 e = Add2Ptr(attr, le16_to_cpu(lrh->attr_off));
>
> +               /*
> +                * e->view.data_off and dlen come from the on-disk
> +                * INDEX_ROOT entry / LRH.  The neighbouring read sites
> +                * (e.g. fs/ntfs3/index.c) check that
> +                * view.data_off + view.data_size <= e->size; mirror that
> +                * bound here so the memmove cannot reach past the entry.
> +                */
> +               if (le16_to_cpu(e->view.data_off) > le16_to_cpu(e->size) ||
> +                   le16_to_cpu(e->view.data_off) + dlen >
> +                           le16_to_cpu(e->size))
> +                       goto dirty_vol;
> +
>                 memmove(Add2Ptr(e, le16_to_cpu(e->view.data_off)), data, dlen);
>
>                 mi->dirty = true;
> @@ -3689,6 +3701,12 @@ static int do_action(struct ntfs_log *log, struct OPEN_ATTR_ENRTY *oe,
>                         goto dirty_vol;
>                 }
>
> +               /* See UpdateRecordDataRoot for the rationale. */
> +               if (le16_to_cpu(e->view.data_off) > le16_to_cpu(e->size) ||
> +                   le16_to_cpu(e->view.data_off) + dlen >
> +                           le16_to_cpu(e->size))
> +                       goto dirty_vol;
> +
>                 memmove(Add2Ptr(e, le16_to_cpu(e->view.data_off)), data, dlen);
>
>                 a_dirty = true;
> --
> 2.53.0
>
Re: [PATCH] fs/ntfs3: bound NTFS_DE view.data_off in UpdateRecordData{Root,Allocation}
Posted by Michael Bommarito 1 week, 1 day ago 
On Sat, May 16, 2026 at 12:44 AM Pavitra Jha <jhapavitra98@gmail.com> wrote:
>
> Thanks for fixing this. Since you cited my original report as the
> basis, would you mind adding:
>
> Reported-by: Pavitra Jha <jhapavitra98@gmail.com>
>
> to the patch tags?

Yes, very sorry about that!  I switched from a 4/4 patch set to
separate patches and missed your attribution.  I'll let Konstantin and
others weigh in on the patch before sending v2, but definitely agree
with you that you should be tagged here.

Thanks,
Mike
[PATCH v2] fs/ntfs3: bound NTFS_DE view.data_off in UpdateRecordData{Root,Allocation}
Posted by Michael Bommarito 5 days, 21 hours ago 
In do_action()'s UpdateRecordDataRoot (fslog.c:3489) and
UpdateRecordDataAllocation (fslog.c:3697) cases, the memmove
destination is `Add2Ptr(e, le16_to_cpu(e->view.data_off))`,
where e->view.data_off comes from an on-disk NTFS_DE inside
an INDEX_ROOT or INDEX_BUFFER.  Neither case validates
view.data_off + dlen against e->size; the existing
check_if_index_root / check_if_alloc_index helpers walk the
entry chain and validate the entry's offset, but not its
internal view fields.

The neighbouring read sites (e.g., fs/ntfs3/index.c when
iterating view entries) check view.data_off + view.data_size
<= e->size.  Apply the same bound at the two memmove sites.

Reproduced under UML+KASAN on mainline 8d90b09e6741 via
pr_warn-only probe instrumentation: with view.data_off forced
to 0xFFFC, the memmove writes 32 bytes past the end of the
NTFS_DE.

This is similar in shape to Pavitra Jha's 2026-05-02 patch
"fs/ntfs3: prevent oob in case UpdateRecordDataRoot"
(<20260502105008.21827-1-jhapavitra98@gmail.com>) which
proposes calling ntfs3_bad_de_range(); that helper does not
exist in mainline.  This patch uses inline checks.

Fixes: b46acd6a6a62 ("fs/ntfs3: Add NTFS journal")
Cc: stable@vger.kernel.org
Reported-by: Pavitra Jha <jhapavitra98@gmail.com>
Closes: https://lore.kernel.org/ntfs3/20260502105008.21827-1-jhapavitra98@gmail.com/
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
v2:
  - Add Pavitra Jha's Reported-by trailer and a Closes tag for
    the original public report, as requested in review.

 fs/ntfs3/fslog.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
index acfa18b84401e..127860fd2ab50 100644
--- a/fs/ntfs3/fslog.c
+++ b/fs/ntfs3/fslog.c
@@ -3497,6 +3497,18 @@ static int do_action(struct ntfs_log *log, struct OPEN_ATTR_ENRTY *oe,
 
 		e = Add2Ptr(attr, le16_to_cpu(lrh->attr_off));
 
+		/*
+		 * e->view.data_off and dlen come from the on-disk
+		 * INDEX_ROOT entry / LRH.  The neighbouring read sites
+		 * (e.g. fs/ntfs3/index.c) check that
+		 * view.data_off + view.data_size <= e->size; mirror that
+		 * bound here so the memmove cannot reach past the entry.
+		 */
+		if (le16_to_cpu(e->view.data_off) > le16_to_cpu(e->size) ||
+		    le16_to_cpu(e->view.data_off) + dlen >
+			    le16_to_cpu(e->size))
+			goto dirty_vol;
+
 		memmove(Add2Ptr(e, le16_to_cpu(e->view.data_off)), data, dlen);
 
 		mi->dirty = true;
@@ -3689,6 +3701,12 @@ static int do_action(struct ntfs_log *log, struct OPEN_ATTR_ENRTY *oe,
 			goto dirty_vol;
 		}
 
+		/* See UpdateRecordDataRoot for the rationale. */
+		if (le16_to_cpu(e->view.data_off) > le16_to_cpu(e->size) ||
+		    le16_to_cpu(e->view.data_off) + dlen >
+			    le16_to_cpu(e->size))
+			goto dirty_vol;
+
 		memmove(Add2Ptr(e, le16_to_cpu(e->view.data_off)), data, dlen);
 
 		a_dirty = true;
-- 
2.53.0

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.