diff mbox series

[SRU,R,v2,1/1] ntfs3: use page allocation for resident attribute inline data

Message ID 20260917192444.26986-2-w.paszajew@gmail.com
State New
Headers show
Series ntfs3: use page allocation for resident attribute inline data (LP: #2165844) | expand

Commit Message

Viktor Pashaiev Sept. 17, 2026, 7:24 p.m. UTC 
From: Namjae Jeon <linkinjeon@kernel.org>

BugLink: https://bugs.launchpad.net/bugs/2165844

SRU Justification:

[ Impact ]

Any buffered write() to an ntfs3-mounted volume can trigger a kernel BUG in
the generic iomap write path and panic the kernel:
  kernel BUG at fs/iomap/buffered-io.c:1061!
  RIP: 0010:iomap_write_end+0x...

This crash is reachable from unprivileged userspace with a plain write(2)
syscall (e.g. file copy, rsync, or appending data). Small files with resident
attribute data allocate inline_data using kmemdup() from slab. Because slab
allocations rarely start at the beginning of a physical page (i.e.
offset_in_page(inline_data) > 0), when the file size plus that offset exceeds
PAGE_SIZE (4096 bytes), iomap_inline_data_valid() fails and hits the BUG_ON
assertion, immediately crashing the system.

[ Fix ]

Backport upstream commit 36ee1313199b7f16bf963c6ac0241861585125d9:
"fs/ntfs3: Use page allocation for resident attribute inline data".

Replace kmemdup() with alloc_page(GFP_NOFS | __GFP_ZERO) so that
page_address() is always strictly page-aligned (offset_in_page == 0).
Update the cleanup path to put_page() accordingly.
For Ubuntu 7.0 (resolute), paths are adjusted from fs/ntfs/ (upstream 7.1+)
to fs/ntfs3/ (Ubuntu 7.0 kernel driver location).

[ Test Plan ]

1. Create and mount an NTFS filesystem:
   mkfs.ntfs -Q -F /dev/loopX
   mount -t ntfs3 /dev/loopX /mnt
2. Perform buffered writes on small resident files near the 4KB boundary
   from an unprivileged account.
   Without the fix: triggers immediate kernel BUG_ON at fs/iomap/buffered-io.c:1061.
   With the fix: write completes successfully without panic, and data integrity
   is verified upon unmount/remount.

[ Where problems could occur ]

The change is strictly isolated to resident attribute handling in fs/ntfs3.
Instead of allocating arbitrary slab chunks, a single dedicated page is
allocated and immediately released via put_page() upon completion of the
iomap operation. The memory overhead is negligible (4KB transient per active
resident write), and non-resident attributes or other filesystems are
completely unaffected.

[ Other Info ]

This fix was requested by Benjamin Wheeler in LP: #2165844.
The backport was verified with independent clean builds on linux-source-7.0.0
against both 7.0.0-31-generic and 7.0.0-30-generic headers, and git am
applies cleanly without offsets or warnings.

(backported from commit 36ee1313199b7f16bf963c6ac0241861585125d9)
[Viktor Pashaiev: adjusted file paths from fs/ntfs/ to fs/ntfs3/ for 7.0]
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Viktor Pashaiev <w.paszajew@gmail.com>
---
 fs/ntfs3/attrib.c | 10 +++++++---
 fs/ntfs3/inode.c  |  6 +++---
 2 files changed, 10 insertions(+), 6 deletions(-)

Comments

Andrei Gherzan Sept. 18, 2026, 11:02 a.m. UTC | #1 
On 2026/09/17 07:24PM, Viktor Pashaiev wrote:
> From: Namjae Jeon <linkinjeon@kernel.org>
> 
> BugLink: https://bugs.launchpad.net/bugs/2165844
> 
> SRU Justification:
> 
> [ Impact ]
> 
> Any buffered write() to an ntfs3-mounted volume can trigger a kernel BUG in
> the generic iomap write path and panic the kernel:
>   kernel BUG at fs/iomap/buffered-io.c:1061!
>   RIP: 0010:iomap_write_end+0x...
> 
> This crash is reachable from unprivileged userspace with a plain write(2)
> syscall (e.g. file copy, rsync, or appending data). Small files with resident
> attribute data allocate inline_data using kmemdup() from slab. Because slab
> allocations rarely start at the beginning of a physical page (i.e.
> offset_in_page(inline_data) > 0), when the file size plus that offset exceeds
> PAGE_SIZE (4096 bytes), iomap_inline_data_valid() fails and hits the BUG_ON
> assertion, immediately crashing the system.
> 
> [ Fix ]
> 
> Backport upstream commit 36ee1313199b7f16bf963c6ac0241861585125d9:
> "fs/ntfs3: Use page allocation for resident attribute inline data".
> 
> Replace kmemdup() with alloc_page(GFP_NOFS | __GFP_ZERO) so that
> page_address() is always strictly page-aligned (offset_in_page == 0).
> Update the cleanup path to put_page() accordingly.
> For Ubuntu 7.0 (resolute), paths are adjusted from fs/ntfs/ (upstream 7.1+)
> to fs/ntfs3/ (Ubuntu 7.0 kernel driver location).
> 
> [ Test Plan ]
> 
> 1. Create and mount an NTFS filesystem:
>    mkfs.ntfs -Q -F /dev/loopX
>    mount -t ntfs3 /dev/loopX /mnt
> 2. Perform buffered writes on small resident files near the 4KB boundary
>    from an unprivileged account.
>    Without the fix: triggers immediate kernel BUG_ON at fs/iomap/buffered-io.c:1061.
>    With the fix: write completes successfully without panic, and data integrity
>    is verified upon unmount/remount.
> 
> [ Where problems could occur ]
> 
> The change is strictly isolated to resident attribute handling in fs/ntfs3.
> Instead of allocating arbitrary slab chunks, a single dedicated page is
> allocated and immediately released via put_page() upon completion of the
> iomap operation. The memory overhead is negligible (4KB transient per active
> resident write), and non-resident attributes or other filesystems are
> completely unaffected.
> 
> [ Other Info ]
> 
> This fix was requested by Benjamin Wheeler in LP: #2165844.
> The backport was verified with independent clean builds on linux-source-7.0.0
> against both 7.0.0-31-generic and 7.0.0-30-generic headers, and git am
> applies cleanly without offsets or warnings.
> 
> (backported from commit 36ee1313199b7f16bf963c6ac0241861585125d9)

The cover letter should only be in the cover email not copied in the
backported/cherry-picked commit. I think I generated this confusion because on
your initial submission the cover letter went in spam.

Also, this fix is already in Resolute:

18c7ba7a1e6d ntfs3: Allocate iomap inline_data using alloc_page
  BugLink: https://bugs.launchpad.net/bugs/2164666
  [ Upstream commit 70d3855594cf6e8791970714b65cac3202d6160e ]
  Fixes: 099ef9ab9203 ("fs/ntfs3: implement iomap-based file operations")

> [Viktor Pashaiev: adjusted file paths from fs/ntfs/ to fs/ntfs3/ for 7.0]
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> Signed-off-by: Viktor Pashaiev <w.paszajew@gmail.com>
> ---
>  fs/ntfs3/attrib.c | 10 +++++++---
>  fs/ntfs3/inode.c  |  6 +++---
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> --- a/fs/ntfs3/attrib.c
> +++ b/fs/ntfs3/attrib.c
> @@ -1042,10 +1042,14 @@ int attr_data_get_block_locked(struct ntfs_inode *ni, CLST vcn, CLST clen,
>  		*lcn = RESIDENT_LCN;
>  		*len = data_size;
>  		if (res && data_size) {
> -			*res = kmemdup(resident_data(attr_b), data_size,
> -				       GFP_KERNEL);
> -			if (!*res)
> +			struct page *page = alloc_page(GFP_NOFS | __GFP_ZERO);
> +			if (page) {
> +				memcpy(page_address(page), resident_data(attr_b),
> +				       data_size);
> +				*res = page;
> +			} else {
>  				err = -ENOMEM;
> +			}
>  		}
>  		goto out;
>  	}
> --- a/fs/ntfs3/inode.c
> +++ b/fs/ntfs3/inode.c
> @@ -845,7 +845,7 @@ static int ntfs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
>  
>  	if (lcn == RESIDENT_LCN) {
>  		if (offset >= clen) {
> -			kfree(res);
> +			put_page((struct page *)res);
>  			if (flags & IOMAP_REPORT) {
>  				/* special code for report. */
>  				return -ENOENT;
> @@ -853,7 +853,8 @@ static int ntfs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
>  			return -EFAULT;
>  		}
>  
> -		iomap->private = iomap->inline_data = res;
> +		iomap->private = res;
> +		iomap->inline_data = page_address((struct page *)res);
>  		iomap->type = IOMAP_INLINE;
>  		iomap->offset = 0;
>  		iomap->length = clen; /* resident size in bytes. */
> @@ -965,7 +966,7 @@ static int ntfs_iomap_end(struct inode *inode, loff_t pos, loff_t length,
>  
>  out:
>  	if (iomap->type == IOMAP_INLINE) {
> -		kfree(iomap->private);
> +		put_page((struct page *)iomap->private);
>  		iomap->private = NULL;
>  	}
>  
> -- 
> 2.43.0
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Viktor Pashaiev Sept. 18, 2026, 11:12 a.m. UTC | #2 
Hi Andrei,

Thanks for the clarification and for checking the Resolute tree.

Good to know that commit 18c7ba7a1e6d already landed via LP: #2164666.
I will follow up on LP: #2165844 to reference that commit and close
out the bug once the kernel lands in proposed.

Also noted regarding the cover letter placement for future kernel submissions.

Thanks!
Viktor Pashaiev

pt., 18 wrz 2026 o 13:02 Andrei Gherzan <andrei.gherzan@canonical.com>
napisał(a):
>
> On 2026/09/17 07:24PM, Viktor Pashaiev wrote:
> > From: Namjae Jeon <linkinjeon@kernel.org>
> >
> > BugLink: https://bugs.launchpad.net/bugs/2165844
> >
> > SRU Justification:
> >
> > [ Impact ]
> >
> > Any buffered write() to an ntfs3-mounted volume can trigger a kernel BUG in
> > the generic iomap write path and panic the kernel:
> >   kernel BUG at fs/iomap/buffered-io.c:1061!
> >   RIP: 0010:iomap_write_end+0x...
> >
> > This crash is reachable from unprivileged userspace with a plain write(2)
> > syscall (e.g. file copy, rsync, or appending data). Small files with resident
> > attribute data allocate inline_data using kmemdup() from slab. Because slab
> > allocations rarely start at the beginning of a physical page (i.e.
> > offset_in_page(inline_data) > 0), when the file size plus that offset exceeds
> > PAGE_SIZE (4096 bytes), iomap_inline_data_valid() fails and hits the BUG_ON
> > assertion, immediately crashing the system.
> >
> > [ Fix ]
> >
> > Backport upstream commit 36ee1313199b7f16bf963c6ac0241861585125d9:
> > "fs/ntfs3: Use page allocation for resident attribute inline data".
> >
> > Replace kmemdup() with alloc_page(GFP_NOFS | __GFP_ZERO) so that
> > page_address() is always strictly page-aligned (offset_in_page == 0).
> > Update the cleanup path to put_page() accordingly.
> > For Ubuntu 7.0 (resolute), paths are adjusted from fs/ntfs/ (upstream 7.1+)
> > to fs/ntfs3/ (Ubuntu 7.0 kernel driver location).
> >
> > [ Test Plan ]
> >
> > 1. Create and mount an NTFS filesystem:
> >    mkfs.ntfs -Q -F /dev/loopX
> >    mount -t ntfs3 /dev/loopX /mnt
> > 2. Perform buffered writes on small resident files near the 4KB boundary
> >    from an unprivileged account.
> >    Without the fix: triggers immediate kernel BUG_ON at fs/iomap/buffered-io.c:1061.
> >    With the fix: write completes successfully without panic, and data integrity
> >    is verified upon unmount/remount.
> >
> > [ Where problems could occur ]
> >
> > The change is strictly isolated to resident attribute handling in fs/ntfs3.
> > Instead of allocating arbitrary slab chunks, a single dedicated page is
> > allocated and immediately released via put_page() upon completion of the
> > iomap operation. The memory overhead is negligible (4KB transient per active
> > resident write), and non-resident attributes or other filesystems are
> > completely unaffected.
> >
> > [ Other Info ]
> >
> > This fix was requested by Benjamin Wheeler in LP: #2165844.
> > The backport was verified with independent clean builds on linux-source-7.0.0
> > against both 7.0.0-31-generic and 7.0.0-30-generic headers, and git am
> > applies cleanly without offsets or warnings.
> >
> > (backported from commit 36ee1313199b7f16bf963c6ac0241861585125d9)
>
> The cover letter should only be in the cover email not copied in the
> backported/cherry-picked commit. I think I generated this confusion because on
> your initial submission the cover letter went in spam.
>
> Also, this fix is already in Resolute:
>
> 18c7ba7a1e6d ntfs3: Allocate iomap inline_data using alloc_page
>   BugLink: https://bugs.launchpad.net/bugs/2164666
>   [ Upstream commit 70d3855594cf6e8791970714b65cac3202d6160e ]
>   Fixes: 099ef9ab9203 ("fs/ntfs3: implement iomap-based file operations")
>
> > [Viktor Pashaiev: adjusted file paths from fs/ntfs/ to fs/ntfs3/ for 7.0]
> > Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> > Signed-off-by: Viktor Pashaiev <w.paszajew@gmail.com>
> > ---
> >  fs/ntfs3/attrib.c | 10 +++++++---
> >  fs/ntfs3/inode.c  |  6 +++---
> >  2 files changed, 10 insertions(+), 6 deletions(-)
> >
> > --- a/fs/ntfs3/attrib.c
> > +++ b/fs/ntfs3/attrib.c
> > @@ -1042,10 +1042,14 @@ int attr_data_get_block_locked(struct ntfs_inode *ni, CLST vcn, CLST clen,
> >               *lcn = RESIDENT_LCN;
> >               *len = data_size;
> >               if (res && data_size) {
> > -                     *res = kmemdup(resident_data(attr_b), data_size,
> > -                                    GFP_KERNEL);
> > -                     if (!*res)
> > +                     struct page *page = alloc_page(GFP_NOFS | __GFP_ZERO);
> > +                     if (page) {
> > +                             memcpy(page_address(page), resident_data(attr_b),
> > +                                    data_size);
> > +                             *res = page;
> > +                     } else {
> >                               err = -ENOMEM;
> > +                     }
> >               }
> >               goto out;
> >       }
> > --- a/fs/ntfs3/inode.c
> > +++ b/fs/ntfs3/inode.c
> > @@ -845,7 +845,7 @@ static int ntfs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
> >
> >       if (lcn == RESIDENT_LCN) {
> >               if (offset >= clen) {
> > -                     kfree(res);
> > +                     put_page((struct page *)res);
> >                       if (flags & IOMAP_REPORT) {
> >                               /* special code for report. */
> >                               return -ENOENT;
> > @@ -853,7 +853,8 @@ static int ntfs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
> >                       return -EFAULT;
> >               }
> >
> > -             iomap->private = iomap->inline_data = res;
> > +             iomap->private = res;
> > +             iomap->inline_data = page_address((struct page *)res);
> >               iomap->type = IOMAP_INLINE;
> >               iomap->offset = 0;
> >               iomap->length = clen; /* resident size in bytes. */
> > @@ -965,7 +966,7 @@ static int ntfs_iomap_end(struct inode *inode, loff_t pos, loff_t length,
> >
> >  out:
> >       if (iomap->type == IOMAP_INLINE) {
> > -             kfree(iomap->private);
> > +             put_page((struct page *)iomap->private);
> >               iomap->private = NULL;
> >       }
> >
> > --
> > 2.43.0
> >
> > --
> > kernel-team mailing list
> > kernel-team@lists.ubuntu.com
> > https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
> --
> Andrei Gherzan
> gpg: rsa4096/D4D94F67AD0E9640
diff mbox series

Patch

--- a/fs/ntfs3/attrib.c
+++ b/fs/ntfs3/attrib.c
@@ -1042,10 +1042,14 @@  int attr_data_get_block_locked(struct ntfs_inode *ni, CLST vcn, CLST clen,
 		*lcn = RESIDENT_LCN;
 		*len = data_size;
 		if (res && data_size) {
-			*res = kmemdup(resident_data(attr_b), data_size,
-				       GFP_KERNEL);
-			if (!*res)
+			struct page *page = alloc_page(GFP_NOFS | __GFP_ZERO);
+			if (page) {
+				memcpy(page_address(page), resident_data(attr_b),
+				       data_size);
+				*res = page;
+			} else {
 				err = -ENOMEM;
+			}
 		}
 		goto out;
 	}
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -845,7 +845,7 @@  static int ntfs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
 
 	if (lcn == RESIDENT_LCN) {
 		if (offset >= clen) {
-			kfree(res);
+			put_page((struct page *)res);
 			if (flags & IOMAP_REPORT) {
 				/* special code for report. */
 				return -ENOENT;
@@ -853,7 +853,8 @@  static int ntfs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
 			return -EFAULT;
 		}
 
-		iomap->private = iomap->inline_data = res;
+		iomap->private = res;
+		iomap->inline_data = page_address((struct page *)res);
 		iomap->type = IOMAP_INLINE;
 		iomap->offset = 0;
 		iomap->length = clen; /* resident size in bytes. */
@@ -965,7 +966,7 @@  static int ntfs_iomap_end(struct inode *inode, loff_t pos, loff_t length,
 
 out:
 	if (iomap->type == IOMAP_INLINE) {
-		kfree(iomap->private);
+		put_page((struct page *)iomap->private);
 		iomap->private = NULL;
 	}