[PATCH] hfsplus: limit sb_maxbytes to partition size

[PATCH] hfsplus: limit sb_maxbytes to partition size

[Hyunchul Lee]

s_maxbytes currently is set to MAX_LFS_FILESIZE,
which allows writes beyond the partition size. As a result,
large-offset writes on small partitions can fail late
with ENOSPC.

Set s_maxbytes to the partition size.

With this change, the large-offset writes are rejected
early by `generic_write_check_limit()` with EFBIG.

This patch also fixes generic/268.

Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com>
---
 fs/hfsplus/super.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index 7229a8ae89f9..18350abc659b 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -500,7 +500,8 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
 
 	/* Set up operations so we can load metadata */
 	sb->s_op = &hfsplus_sops;
-	sb->s_maxbytes = MAX_LFS_FILESIZE;
+	sb->s_maxbytes = (loff_t)min(MAX_LFS_FILESIZE,
+				     (u64)sbi->total_blocks << sbi->alloc_blksz_shift);
 
 	if (!(vhdr->attributes & cpu_to_be32(HFSPLUS_VOL_UNMNT))) {
 		pr_warn("Filesystem was not cleanly unmounted, running fsck.hfsplus is recommended.  mounting read-only.\n");
-- 
2.43.0

Re: [PATCH] hfsplus: limit sb_maxbytes to partition size

[Christoph Hellwig]

On Tue, Mar 03, 2026 at 05:28:07PM +0900, Hyunchul Lee wrote:
> s_maxbytes currently is set to MAX_LFS_FILESIZE,
> which allows writes beyond the partition size.

The "partition size" does not matter here.  s_maxbytes is the maximum
size supported by the format and has nothing to do with the actual space
allocated to the file system (which in Linux terminology would be the
block device and not the partition anyway).

>
> As a result,
> large-offset writes on small partitions can fail late
> with ENOSPC.

That sounds like some other check is missing in hfsplus, but it
should be about the available free space, not the device size.

RE: [PATCH] hfsplus: limit sb_maxbytes to partition size

[Viacheslav Dubeyko]

On Wed, 2026-03-04 at 05:08 -0800, Christoph Hellwig wrote:
> On Tue, Mar 03, 2026 at 05:28:07PM +0900, Hyunchul Lee wrote:
> > s_maxbytes currently is set to MAX_LFS_FILESIZE,
> > which allows writes beyond the partition size.
> 
> The "partition size" does not matter here.  s_maxbytes is the maximum
> size supported by the format and has nothing to do with the actual space
> allocated to the file system (which in Linux terminology would be the
> block device and not the partition anyway).
> 
> > 
> > As a result,
> > large-offset writes on small partitions can fail late
> > with ENOSPC.
> 
> That sounds like some other check is missing in hfsplus, but it
> should be about the available free space, not the device size.
> 

I agree with Christoph.

But, frankly speaking, I don't quite follow which particular issue is under fix
here. I can see that generic/268 failure has been mentioned. However, I can see
this:

sudo ./check generic/268 
FSTYP         -- hfsplus
PLATFORM      -- Linux/x86_64 hfsplus-testing-0001 6.19.0-rc1+ #95 SMP
PREEMPT_DYNAMIC Thu Feb 19 15:29:55 PST 2026
MKFS_OPTIONS  -- /dev/loop51
MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch

generic/268       [not run] Reflink not supported by scratch filesystem type:
hfsplus
Ran: generic/268
Not run: generic/268
Passed all 1 tests

Which particular issue is under fix?

Thanks,
Slava.

Re: [PATCH] hfsplus: limit sb_maxbytes to partition size

[Hyunchul Lee]

On Wed, Mar 04, 2026 at 05:08:15AM -0800, Christoph Hellwig wrote:
> On Tue, Mar 03, 2026 at 05:28:07PM +0900, Hyunchul Lee wrote:
> > s_maxbytes currently is set to MAX_LFS_FILESIZE,
> > which allows writes beyond the partition size.
> 
> The "partition size" does not matter here.  s_maxbytes is the maximum
> size supported by the format and has nothing to do with the actual space
> allocated to the file system (which in Linux terminology would be the
> block device and not the partition anyway).
> 
> >
> > As a result,
> > large-offset writes on small partitions can fail late
> > with ENOSPC.
> 
> That sounds like some other check is missing in hfsplus, but it
> should be about the available free space, not the device size.
> 

When running xfs_io -c "pwrite 8t 512", hfsplus fills the block device
with zeros before returning ENOSPC. I was trying to fix this,
but as you mentioned, I will look for another solution.

Thank for your review and comments.

-- 
Thanks,
Hyunchul

Re: [PATCH] hfsplus: limit sb_maxbytes to partition size

[Hyunchul Lee]

On Wed, Mar 04, 2026 at 08:04:30PM +0000, Viacheslav Dubeyko wrote:
> On Wed, 2026-03-04 at 05:08 -0800, Christoph Hellwig wrote:
> > On Tue, Mar 03, 2026 at 05:28:07PM +0900, Hyunchul Lee wrote:
> > > s_maxbytes currently is set to MAX_LFS_FILESIZE,
> > > which allows writes beyond the partition size.
> > 
> > The "partition size" does not matter here.  s_maxbytes is the maximum
> > size supported by the format and has nothing to do with the actual space
> > allocated to the file system (which in Linux terminology would be the
> > block device and not the partition anyway).
> > 
> > > 
> > > As a result,
> > > large-offset writes on small partitions can fail late
> > > with ENOSPC.
> > 
> > That sounds like some other check is missing in hfsplus, but it
> > should be about the available free space, not the device size.
> > 
> 
> I agree with Christoph.
> 
> But, frankly speaking, I don't quite follow which particular issue is under fix
> here. I can see that generic/268 failure has been mentioned. However, I can see
> this:
> 
> sudo ./check generic/268 
> FSTYP         -- hfsplus
> PLATFORM      -- Linux/x86_64 hfsplus-testing-0001 6.19.0-rc1+ #95 SMP
> PREEMPT_DYNAMIC Thu Feb 19 15:29:55 PST 2026
> MKFS_OPTIONS  -- /dev/loop51
> MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch
> 
> generic/268       [not run] Reflink not supported by scratch filesystem type:
> hfsplus
> Ran: generic/268
> Not run: generic/268
> Passed all 1 tests
> 
> Which particular issue is under fix?

Sorry it's generic/285, not generic/268.
in generic/285, there is a test that creates a hole exceeding the block
size and appends small data to the file. hfsplus fails because it fills
the block device and returns ENOSPC. However if it returns EFBIG
instead, the test is skipped.

For writes like xfs_io -c "pwrite 8t 512", should fops->write_iter
returns ENOSPC, or would it be better to return EFBIG?

> 
> Thanks,
> Slava.

-- 
Thanks,
Hyunchul

RE: [PATCH] hfsplus: limit sb_maxbytes to partition size

[Viacheslav Dubeyko]

On Thu, 2026-03-05 at 09:29 +0900, Hyunchul Lee wrote:
> On Wed, Mar 04, 2026 at 08:04:30PM +0000, Viacheslav Dubeyko wrote:
> > On Wed, 2026-03-04 at 05:08 -0800, Christoph Hellwig wrote:
> > > On Tue, Mar 03, 2026 at 05:28:07PM +0900, Hyunchul Lee wrote:
> > > > s_maxbytes currently is set to MAX_LFS_FILESIZE,
> > > > which allows writes beyond the partition size.
> > > 
> > > The "partition size" does not matter here.  s_maxbytes is the maximum
> > > size supported by the format and has nothing to do with the actual space
> > > allocated to the file system (which in Linux terminology would be the
> > > block device and not the partition anyway).
> > > 
> > > > 
> > > > As a result,
> > > > large-offset writes on small partitions can fail late
> > > > with ENOSPC.
> > > 
> > > That sounds like some other check is missing in hfsplus, but it
> > > should be about the available free space, not the device size.
> > > 
> > 
> > I agree with Christoph.
> > 
> > But, frankly speaking, I don't quite follow which particular issue is under fix
> > here. I can see that generic/268 failure has been mentioned. However, I can see
> > this:
> > 
> > sudo ./check generic/268 
> > FSTYP         -- hfsplus
> > PLATFORM      -- Linux/x86_64 hfsplus-testing-0001 6.19.0-rc1+ #95 SMP
> > PREEMPT_DYNAMIC Thu Feb 19 15:29:55 PST 2026
> > MKFS_OPTIONS  -- /dev/loop51
> > MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch
> > 
> > generic/268       [not run] Reflink not supported by scratch filesystem type:
> > hfsplus
> > Ran: generic/268
> > Not run: generic/268
> > Passed all 1 tests
> > 
> > Which particular issue is under fix?
> 
> Sorry it's generic/285, not generic/268.
> in generic/285, there is a test that creates a hole exceeding the block
> size and appends small data to the file. hfsplus fails because it fills
> the block device and returns ENOSPC. However if it returns EFBIG
> instead, the test is skipped.
> 
> For writes like xfs_io -c "pwrite 8t 512", should fops->write_iter
> returns ENOSPC, or would it be better to return EFBIG?
> > 

Current hfsplus_file_extend() implementation doesn't support holes. I assume you
mean this code [1]:

	len = hip->clump_blocks;
	start = hfsplus_block_allocate(sb, sbi->total_blocks, goal, &len);
	if (start >= sbi->total_blocks) {
		start = hfsplus_block_allocate(sb, goal, 0, &len);
		if (start >= goal) {
			res = -ENOSPC;
			goto out;
		}
	}

Am I correct?

Do you mean that calling logic expects -EFBIG? Potentially, if we tries to
extend the file, then -EFBIG could be more appropriate. But it needs to check
the whole call trace.

Thanks,
Slava.

[1] https://elixir.bootlin.com/linux/v6.19/source/fs/hfsplus/extents.c#L463

Re: [PATCH] hfsplus: limit sb_maxbytes to partition size

[Hyunchul Lee]

2026년 3월 5일 (목) AM 9:47, Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>님이 작성:
>
> On Thu, 2026-03-05 at 09:29 +0900, Hyunchul Lee wrote:
> > On Wed, Mar 04, 2026 at 08:04:30PM +0000, Viacheslav Dubeyko wrote:
> > > On Wed, 2026-03-04 at 05:08 -0800, Christoph Hellwig wrote:
> > > > On Tue, Mar 03, 2026 at 05:28:07PM +0900, Hyunchul Lee wrote:
> > > > > s_maxbytes currently is set to MAX_LFS_FILESIZE,
> > > > > which allows writes beyond the partition size.
> > > >
> > > > The "partition size" does not matter here.  s_maxbytes is the maximum
> > > > size supported by the format and has nothing to do with the actual space
> > > > allocated to the file system (which in Linux terminology would be the
> > > > block device and not the partition anyway).
> > > >
> > > > >
> > > > > As a result,
> > > > > large-offset writes on small partitions can fail late
> > > > > with ENOSPC.
> > > >
> > > > That sounds like some other check is missing in hfsplus, but it
> > > > should be about the available free space, not the device size.
> > > >
> > >
> > > I agree with Christoph.
> > >
> > > But, frankly speaking, I don't quite follow which particular issue is under fix
> > > here. I can see that generic/268 failure has been mentioned. However, I can see
> > > this:
> > >
> > > sudo ./check generic/268
> > > FSTYP         -- hfsplus
> > > PLATFORM      -- Linux/x86_64 hfsplus-testing-0001 6.19.0-rc1+ #95 SMP
> > > PREEMPT_DYNAMIC Thu Feb 19 15:29:55 PST 2026
> > > MKFS_OPTIONS  -- /dev/loop51
> > > MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch
> > >
> > > generic/268       [not run] Reflink not supported by scratch filesystem type:
> > > hfsplus
> > > Ran: generic/268
> > > Not run: generic/268
> > > Passed all 1 tests
> > >
> > > Which particular issue is under fix?
> >
> > Sorry it's generic/285, not generic/268.
> > in generic/285, there is a test that creates a hole exceeding the block
> > size and appends small data to the file. hfsplus fails because it fills
> > the block device and returns ENOSPC. However if it returns EFBIG
> > instead, the test is skipped.
> >
> > For writes like xfs_io -c "pwrite 8t 512", should fops->write_iter
> > returns ENOSPC, or would it be better to return EFBIG?
> > >
>
> Current hfsplus_file_extend() implementation doesn't support holes. I assume you
> mean this code [1]:
>
>         len = hip->clump_blocks;
>         start = hfsplus_block_allocate(sb, sbi->total_blocks, goal, &len);
>         if (start >= sbi->total_blocks) {
>                 start = hfsplus_block_allocate(sb, goal, 0, &len);
>                 if (start >= goal) {
>                         res = -ENOSPC;
>                         goto out;
>                 }
>         }
>
> Am I correct?
>
Yes,

hfsplus_write_begin()
  cont_write_begin()
    cont_expand_zero()

1) xfs_io -c "pwrite 8t 512"
2) hfsplus_begin_write() is called with offset 2^43 and length 512
3) cont_expand_zero() allocates and zeroes out one block repeatedly
for the range
0 to 2^43 - 1. To achieve this, hfsplus_write_begin() is called repeatedly.
4) hfsplus_write_begin() allocates one block through hfsplus_get_block() =>
hfsplus_file_extend()

> Do you mean that calling logic expects -EFBIG? Potentially, if we tries to
> extend the file, then -EFBIG could be more appropriate. But it needs to check
> the whole call trace.

generic/285 creates a hole by pwrite at offset 2^43 + @ and handle the
error as follow:
https://github.com/kdave/xfstests/blob/master/src/seek_sanity_test.c#L271

if (errno == EFBIG) {
  fprintf(stdout, "Test skipped as fs doesn't support so large files.\n");
  ret = 0

>
> Thanks,
> Slava.
>
> [1] https://elixir.bootlin.com/linux/v6.19/source/fs/hfsplus/extents.c#L463



-- 
Thanks,
Hyunchul

Re: [PATCH] hfsplus: limit sb_maxbytes to partition size

[hch]

On Thu, Mar 05, 2026 at 09:29:33AM +0900, Hyunchul Lee wrote:
> Sorry it's generic/285, not generic/268.
> in generic/285, there is a test that creates a hole exceeding the block
> size and appends small data to the file. hfsplus fails because it fills
> the block device and returns ENOSPC. However if it returns EFBIG
> instead, the test is skipped.

generic/285 needs to call _require_sparse_files.

RE: [PATCH] hfsplus: limit sb_maxbytes to partition size

[Viacheslav Dubeyko]

On Thu, 2026-03-05 at 10:52 +0900, Hyunchul Lee wrote:
> > > 
> > > Sorry it's generic/285, not generic/268.
> > > in generic/285, there is a test that creates a hole exceeding the block
> > > size and appends small data to the file. hfsplus fails because it fills
> > > the block device and returns ENOSPC. However if it returns EFBIG
> > > instead, the test is skipped.
> > > 
> > > For writes like xfs_io -c "pwrite 8t 512", should fops->write_iter
> > > returns ENOSPC, or would it be better to return EFBIG?
> > > > 
> > 
> > Current hfsplus_file_extend() implementation doesn't support holes. I assume you
> > mean this code [1]:
> > 
> >         len = hip->clump_blocks;
> >         start = hfsplus_block_allocate(sb, sbi->total_blocks, goal, &len);
> >         if (start >= sbi->total_blocks) {
> >                 start = hfsplus_block_allocate(sb, goal, 0, &len);
> >                 if (start >= goal) {
> >                         res = -ENOSPC;
> >                         goto out;
> >                 }
> >         }
> > 
> > Am I correct?
> > 
> Yes,
> 
> hfsplus_write_begin()
>   cont_write_begin()
>     cont_expand_zero()
> 
> 1) xfs_io -c "pwrite 8t 512"
> 2) hfsplus_begin_write() is called with offset 2^43 and length 512
> 3) cont_expand_zero() allocates and zeroes out one block repeatedly
> for the range
> 0 to 2^43 - 1. To achieve this, hfsplus_write_begin() is called repeatedly.
> 4) hfsplus_write_begin() allocates one block through hfsplus_get_block() =>
> hfsplus_file_extend()

I think we can consider these directions:

(1) Currently, HFS+ code doesn't support holes. So, it means that
hfsplus_write_begin() can check pos variable and i_size_read(inode). If pos is
bigger than i_size_read(inode), then hfsplus_file_extend() will reject such
request. So, we can return error code (probably, -EFBIG) for this case without
calling hfsplus_file_extend(). But, from another point of view, maybe,
hfsplus_file_extend() could be one place for this check. Does it make sense?

(2) I think that hfsplus_file_extend() could treat hole or absence of free
blocks like -ENOSPC. Probably, we can change the error code from -ENOSPC to -
EFBIG in hfsplus_write_begin(). What do you think?

> 
> > Do you mean that calling logic expects -EFBIG? Potentially, if we tries to
> > extend the file, then -EFBIG could be more appropriate. But it needs to check
> > the whole call trace.
> 
> generic/285 creates a hole by pwrite at offset 2^43 + @ and handle the
> error as follow:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_kdave_xfstests_blob_master_src_seek-5Fsanity-5Ftest.c-23L271&d=DwIFaQ&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=84S8DZyqlgcJA0uzXVPYD-cvdonhvyi5kMWaklKdNjD8otp-dvtHXuL2O2CridFV&s=6jI_AQCduo5Tim8ioI5V8Xy50jguCLUTx1CSFEF__D0&e= 
> 
> if (errno == EFBIG) {
>   fprintf(stdout, "Test skipped as fs doesn't support so large files.\n");
>   ret = 0
> 

I believe we need follow to system call documentation but not what some
particular script expects to see. :) But -EFBIG sounds like reasonable error
code.

Thanks,
Slava.

> 

Re: [PATCH] hfsplus: limit sb_maxbytes to partition size

[Hyunchul Lee]

On Thu, Mar 05, 2026 at 06:27:34AM -0800, hch@infradead.org wrote:
> On Thu, Mar 05, 2026 at 09:29:33AM +0900, Hyunchul Lee wrote:
> > Sorry it's generic/285, not generic/268.
> > in generic/285, there is a test that creates a hole exceeding the block
> > size and appends small data to the file. hfsplus fails because it fills
> > the block device and returns ENOSPC. However if it returns EFBIG
> > instead, the test is skipped.
> 
> generic/285 needs to call _require_sparse_files.
> 

The generic/258(src/seek_sanity_test.c) is considering filesystems
that don't support sparse files[1].

int test_basic_support()
  ...
  pos = lseek(fd, 0, SEEK_HOLE);
  ...
  if (pos == filsze) {
    default_behavior = 1;
    fprintf(stderr, "File system supports the default behavior.\n");
  ...

The issue is that there are some tests which write to offsets larger
than the block device. How about skipping for such test cases when
dealing with filesystems that don't support sparse files?

[1]: https://github.com/kdave/xfstests/blob/master/src/seek_sanity_test.c#L1244

-- 
Thanks,
Hyunchul

Re: [PATCH] hfsplus: limit sb_maxbytes to partition size

[Hyunchul Lee]

On Thu, Mar 05, 2026 at 11:21:19PM +0000, Viacheslav Dubeyko wrote:
> On Thu, 2026-03-05 at 10:52 +0900, Hyunchul Lee wrote:
> > > > 
> > > > Sorry it's generic/285, not generic/268.
> > > > in generic/285, there is a test that creates a hole exceeding the block
> > > > size and appends small data to the file. hfsplus fails because it fills
> > > > the block device and returns ENOSPC. However if it returns EFBIG
> > > > instead, the test is skipped.
> > > > 
> > > > For writes like xfs_io -c "pwrite 8t 512", should fops->write_iter
> > > > returns ENOSPC, or would it be better to return EFBIG?
> > > > > 
> > > 
> > > Current hfsplus_file_extend() implementation doesn't support holes. I assume you
> > > mean this code [1]:
> > > 
> > >         len = hip->clump_blocks;
> > >         start = hfsplus_block_allocate(sb, sbi->total_blocks, goal, &len);
> > >         if (start >= sbi->total_blocks) {
> > >                 start = hfsplus_block_allocate(sb, goal, 0, &len);
> > >                 if (start >= goal) {
> > >                         res = -ENOSPC;
> > >                         goto out;
> > >                 }
> > >         }
> > > 
> > > Am I correct?
> > > 
> > Yes,
> > 
> > hfsplus_write_begin()
> >   cont_write_begin()
> >     cont_expand_zero()
> > 
> > 1) xfs_io -c "pwrite 8t 512"
> > 2) hfsplus_begin_write() is called with offset 2^43 and length 512
> > 3) cont_expand_zero() allocates and zeroes out one block repeatedly
> > for the range
> > 0 to 2^43 - 1. To achieve this, hfsplus_write_begin() is called repeatedly.
> > 4) hfsplus_write_begin() allocates one block through hfsplus_get_block() =>
> > hfsplus_file_extend()
> 
> I think we can consider these directions:
> 
> (1) Currently, HFS+ code doesn't support holes. So, it means that
> hfsplus_write_begin() can check pos variable and i_size_read(inode). If pos is
> bigger than i_size_read(inode), then hfsplus_file_extend() will reject such
> request. So, we can return error code (probably, -EFBIG) for this case without
> calling hfsplus_file_extend(). But, from another point of view, maybe,
> hfsplus_file_extend() could be one place for this check. Does it make sense?
> 
> (2) I think that hfsplus_file_extend() could treat hole or absence of free
> blocks like -ENOSPC. Probably, we can change the error code from -ENOSPC to -
> EFBIG in hfsplus_write_begin(). What do you think?
> 
Even if holes are not supported, shouldn't the following writes be
supported?

xfs_io -f -c "pwrite 4k 512" <file-path>

If so, since we need to support cases where pos > i_size_read(inode),
wouldn't the condition "pos - i_size_read(inode) > free space" be better?
Also instead of checking every time in hfsplus_write_begin() or
hfsplus_file_extend(), how about implementing the check in the
file_operations->write_iter callback function, and returing EFBIG?

> > 
> > > Do you mean that calling logic expects -EFBIG? Potentially, if we tries to
> > > extend the file, then -EFBIG could be more appropriate. But it needs to check
> > > the whole call trace.
> > 
> > generic/285 creates a hole by pwrite at offset 2^43 + @ and handle the
> > error as follow:
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_kdave_xfstests_blob_master_src_seek-5Fsanity-5Ftest.c-23L271&d=DwIFaQ&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=84S8DZyqlgcJA0uzXVPYD-cvdonhvyi5kMWaklKdNjD8otp-dvtHXuL2O2CridFV&s=6jI_AQCduo5Tim8ioI5V8Xy50jguCLUTx1CSFEF__D0&e= 
> > 
> > if (errno == EFBIG) {
> >   fprintf(stdout, "Test skipped as fs doesn't support so large files.\n");
> >   ret = 0
> > 
> 
> I believe we need follow to system call documentation but not what some
> particular script expects to see. :) But -EFBIG sounds like reasonable error
> code.
> 

I agree.

> Thanks,
> Slava.
> 
> > 

-- 
Thanks,
Hyunchul

RE: [PATCH] hfsplus: limit sb_maxbytes to partition size

[Viacheslav Dubeyko]

On Fri, 2026-03-06 at 09:57 +0900, Hyunchul Lee wrote:
> On Thu, Mar 05, 2026 at 11:21:19PM +0000, Viacheslav Dubeyko wrote:
> > On Thu, 2026-03-05 at 10:52 +0900, Hyunchul Lee wrote:
> > > > > 
> > > > > Sorry it's generic/285, not generic/268.
> > > > > in generic/285, there is a test that creates a hole exceeding the block
> > > > > size and appends small data to the file. hfsplus fails because it fills
> > > > > the block device and returns ENOSPC. However if it returns EFBIG
> > > > > instead, the test is skipped.
> > > > > 
> > > > > For writes like xfs_io -c "pwrite 8t 512", should fops->write_iter
> > > > > returns ENOSPC, or would it be better to return EFBIG?
> > > > > > 
> > > > 
> > > > Current hfsplus_file_extend() implementation doesn't support holes. I assume you
> > > > mean this code [1]:
> > > > 
> > > >         len = hip->clump_blocks;
> > > >         start = hfsplus_block_allocate(sb, sbi->total_blocks, goal, &len);
> > > >         if (start >= sbi->total_blocks) {
> > > >                 start = hfsplus_block_allocate(sb, goal, 0, &len);
> > > >                 if (start >= goal) {
> > > >                         res = -ENOSPC;
> > > >                         goto out;
> > > >                 }
> > > >         }
> > > > 
> > > > Am I correct?
> > > > 
> > > Yes,
> > > 
> > > hfsplus_write_begin()
> > >   cont_write_begin()
> > >     cont_expand_zero()
> > > 
> > > 1) xfs_io -c "pwrite 8t 512"
> > > 2) hfsplus_begin_write() is called with offset 2^43 and length 512
> > > 3) cont_expand_zero() allocates and zeroes out one block repeatedly
> > > for the range
> > > 0 to 2^43 - 1. To achieve this, hfsplus_write_begin() is called repeatedly.
> > > 4) hfsplus_write_begin() allocates one block through hfsplus_get_block() =>
> > > hfsplus_file_extend()
> > 
> > I think we can consider these directions:
> > 
> > (1) Currently, HFS+ code doesn't support holes. So, it means that
> > hfsplus_write_begin() can check pos variable and i_size_read(inode). If pos is
> > bigger than i_size_read(inode), then hfsplus_file_extend() will reject such
> > request. So, we can return error code (probably, -EFBIG) for this case without
> > calling hfsplus_file_extend(). But, from another point of view, maybe,
> > hfsplus_file_extend() could be one place for this check. Does it make sense?
> > 
> > (2) I think that hfsplus_file_extend() could treat hole or absence of free
> > blocks like -ENOSPC. Probably, we can change the error code from -ENOSPC to -
> > EFBIG in hfsplus_write_begin(). What do you think?
> > 
> Even if holes are not supported, shouldn't the following writes be
> supported?
> 
> xfs_io -f -c "pwrite 4k 512" <file-path>
> 
> If so, since we need to support cases where pos > i_size_read(inode),

The pos > i_size_read(inode) means that you create the hole. Because,
oppositely, when HFS+ logic tries to allocate new block, then it expects to have
pos == i_size_read(inode). And we need to take into account this code [1]:

	if (iblock >= hip->fs_blocks) {
		if (!create)
			return 0;
		if (iblock > hip->fs_blocks) <-- This is the rejection of hole
			return -EIO;
		if (ablock >= hip->alloc_blocks) {
			res = hfsplus_file_extend(inode, false);
			if (res)
				return res;
		}
	}

The generic_write_end() changes the inode size: i_size_write(inode, pos +
copied).

> wouldn't the condition "pos - i_size_read(inode) > free space" be better?
> Also instead of checking every time in hfsplus_write_begin() or
> hfsplus_file_extend(), how about implementing the check in the
> file_operations->write_iter callback function, and returing EFBIG?

Which callback do you mean here? I am not sure that it's good idea.

Thanks,
Slava.

> 
> > > 
> > > 

[1] https://elixir.bootlin.com/linux/v6.19/source/fs/hfsplus/extents.c#L239

Re: [PATCH] hfsplus: limit sb_maxbytes to partition size

[Hyunchul Lee]

On Fri, Mar 06, 2026 at 01:23:16AM +0000, Viacheslav Dubeyko wrote:
> On Fri, 2026-03-06 at 09:57 +0900, Hyunchul Lee wrote:
> > On Thu, Mar 05, 2026 at 11:21:19PM +0000, Viacheslav Dubeyko wrote:
> > > On Thu, 2026-03-05 at 10:52 +0900, Hyunchul Lee wrote:
> > > > > > 
> > > > > > Sorry it's generic/285, not generic/268.
> > > > > > in generic/285, there is a test that creates a hole exceeding the block
> > > > > > size and appends small data to the file. hfsplus fails because it fills
> > > > > > the block device and returns ENOSPC. However if it returns EFBIG
> > > > > > instead, the test is skipped.
> > > > > > 
> > > > > > For writes like xfs_io -c "pwrite 8t 512", should fops->write_iter
> > > > > > returns ENOSPC, or would it be better to return EFBIG?
> > > > > > > 
> > > > > 
> > > > > Current hfsplus_file_extend() implementation doesn't support holes. I assume you
> > > > > mean this code [1]:
> > > > > 
> > > > >         len = hip->clump_blocks;
> > > > >         start = hfsplus_block_allocate(sb, sbi->total_blocks, goal, &len);
> > > > >         if (start >= sbi->total_blocks) {
> > > > >                 start = hfsplus_block_allocate(sb, goal, 0, &len);
> > > > >                 if (start >= goal) {
> > > > >                         res = -ENOSPC;
> > > > >                         goto out;
> > > > >                 }
> > > > >         }
> > > > > 
> > > > > Am I correct?
> > > > > 
> > > > Yes,
> > > > 
> > > > hfsplus_write_begin()
> > > >   cont_write_begin()
> > > >     cont_expand_zero()
> > > > 
> > > > 1) xfs_io -c "pwrite 8t 512"
> > > > 2) hfsplus_begin_write() is called with offset 2^43 and length 512
> > > > 3) cont_expand_zero() allocates and zeroes out one block repeatedly
> > > > for the range
> > > > 0 to 2^43 - 1. To achieve this, hfsplus_write_begin() is called repeatedly.
> > > > 4) hfsplus_write_begin() allocates one block through hfsplus_get_block() =>
> > > > hfsplus_file_extend()
> > > 
> > > I think we can consider these directions:
> > > 
> > > (1) Currently, HFS+ code doesn't support holes. So, it means that
> > > hfsplus_write_begin() can check pos variable and i_size_read(inode). If pos is
> > > bigger than i_size_read(inode), then hfsplus_file_extend() will reject such
> > > request. So, we can return error code (probably, -EFBIG) for this case without
> > > calling hfsplus_file_extend(). But, from another point of view, maybe,
> > > hfsplus_file_extend() could be one place for this check. Does it make sense?
> > > 
> > > (2) I think that hfsplus_file_extend() could treat hole or absence of free
> > > blocks like -ENOSPC. Probably, we can change the error code from -ENOSPC to -
> > > EFBIG in hfsplus_write_begin(). What do you think?
> > > 
> > Even if holes are not supported, shouldn't the following writes be
> > supported?
> > 
> > xfs_io -f -c "pwrite 4k 512" <file-path>
> > 
> > If so, since we need to support cases where pos > i_size_read(inode),
> 
> The pos > i_size_read(inode) means that you create the hole. Because,

That's correct. However I believe that not supporting writes like the
one mentioned above is a significant limitation. Filesystems that don't
support sparse files, such as exFAT, allocate blocks and fill them with
zeros.

> oppositely, when HFS+ logic tries to allocate new block, then it expects to have
> pos == i_size_read(inode). And we need to take into account this code [1]:
> 
> 	if (iblock >= hip->fs_blocks) {
> 		if (!create)
> 			return 0;
> 		if (iblock > hip->fs_blocks) <-- This is the rejection of hole
> 			return -EIO;
> 		if (ablock >= hip->alloc_blocks) {
> 			res = hfsplus_file_extend(inode, false);
> 			if (res)
> 				return res;
> 		}
> 	}
> 
> The generic_write_end() changes the inode size: i_size_write(inode, pos +
> copied).

I think that it's not problem.

hfsplus_write_begin()
  cont_write_begin()
    cont_expand_zero()

cont_expand_zero() calls hfsplus_get_block() to allocate blocks between
i_size_read(inode) and pos, if pos > i_size_read(inode).

> 
> > wouldn't the condition "pos - i_size_read(inode) > free space" be better?
> > Also instead of checking every time in hfsplus_write_begin() or
> > hfsplus_file_extend(), how about implementing the check in the
> > file_operations->write_iter callback function, and returing EFBIG?
> 
> Which callback do you mean here? I am not sure that it's good idea.
> 

Here is a simple code snippet.

 static const struct file_operations hfsplus_file_operations = {
...
-       .write_iter     = generic_file_write_iter,
+       .write_iter     = hfsplus_file_write_iter,
...

+ssize_t hfsplus_file_write_iter(struct kiocb *iocb, struct iov_iter *iter)
+{
...
+       // check iocb->ki_pos is beyond i_size
+
+       ret = generic_file_write_iter(iocb, iter);

> Thanks,
> Slava.
> 
> > 
> > > > 
> > > > 
> 
> [1] https://elixir.bootlin.com/linux/v6.19/source/fs/hfsplus/extents.c#L239

-- 
Thanks,
Hyunchul

RE: [PATCH] hfsplus: limit sb_maxbytes to partition size

[Viacheslav Dubeyko]

On Fri, 2026-03-06 at 11:05 +0900, Hyunchul Lee wrote:
> On Fri, Mar 06, 2026 at 01:23:16AM +0000, Viacheslav Dubeyko wrote:
> > On Fri, 2026-03-06 at 09:57 +0900, Hyunchul Lee wrote:
> > > On Thu, Mar 05, 2026 at 11:21:19PM +0000, Viacheslav Dubeyko wrote:
> > > > On Thu, 2026-03-05 at 10:52 +0900, Hyunchul Lee wrote:
> > > > > > > 
> > > > > > > Sorry it's generic/285, not generic/268.
> > > > > > > in generic/285, there is a test that creates a hole exceeding the block
> > > > > > > size and appends small data to the file. hfsplus fails because it fills
> > > > > > > the block device and returns ENOSPC. However if it returns EFBIG
> > > > > > > instead, the test is skipped.
> > > > > > > 
> > > > > > > For writes like xfs_io -c "pwrite 8t 512", should fops->write_iter
> > > > > > > returns ENOSPC, or would it be better to return EFBIG?
> > > > > > > > 
> > > > > > 
> > > > > > Current hfsplus_file_extend() implementation doesn't support holes. I assume you
> > > > > > mean this code [1]:
> > > > > > 
> > > > > >         len = hip->clump_blocks;
> > > > > >         start = hfsplus_block_allocate(sb, sbi->total_blocks, goal, &len);
> > > > > >         if (start >= sbi->total_blocks) {
> > > > > >                 start = hfsplus_block_allocate(sb, goal, 0, &len);
> > > > > >                 if (start >= goal) {
> > > > > >                         res = -ENOSPC;
> > > > > >                         goto out;
> > > > > >                 }
> > > > > >         }
> > > > > > 
> > > > > > Am I correct?
> > > > > > 
> > > > > Yes,
> > > > > 
> > > > > hfsplus_write_begin()
> > > > >   cont_write_begin()
> > > > >     cont_expand_zero()
> > > > > 
> > > > > 1) xfs_io -c "pwrite 8t 512"
> > > > > 2) hfsplus_begin_write() is called with offset 2^43 and length 512
> > > > > 3) cont_expand_zero() allocates and zeroes out one block repeatedly
> > > > > for the range
> > > > > 0 to 2^43 - 1. To achieve this, hfsplus_write_begin() is called repeatedly.
> > > > > 4) hfsplus_write_begin() allocates one block through hfsplus_get_block() =>
> > > > > hfsplus_file_extend()
> > > > 
> > > > I think we can consider these directions:
> > > > 
> > > > (1) Currently, HFS+ code doesn't support holes. So, it means that
> > > > hfsplus_write_begin() can check pos variable and i_size_read(inode). If pos is
> > > > bigger than i_size_read(inode), then hfsplus_file_extend() will reject such
> > > > request. So, we can return error code (probably, -EFBIG) for this case without
> > > > calling hfsplus_file_extend(). But, from another point of view, maybe,
> > > > hfsplus_file_extend() could be one place for this check. Does it make sense?
> > > > 
> > > > (2) I think that hfsplus_file_extend() could treat hole or absence of free
> > > > blocks like -ENOSPC. Probably, we can change the error code from -ENOSPC to -
> > > > EFBIG in hfsplus_write_begin(). What do you think?
> > > > 
> > > Even if holes are not supported, shouldn't the following writes be
> > > supported?
> > > 
> > > xfs_io -f -c "pwrite 4k 512" <file-path>
> > > 
> > > If so, since we need to support cases where pos > i_size_read(inode),
> > 
> > The pos > i_size_read(inode) means that you create the hole. Because,
> 
> That's correct. However I believe that not supporting writes like the
> one mentioned above is a significant limitation. Filesystems that don't
> support sparse files, such as exFAT, allocate blocks and fill them with
> zeros.
> 

You are welcomed to write the code for HFS/HFS+. :) I'll be happy to see such
support.

> > oppositely, when HFS+ logic tries to allocate new block, then it expects to have
> > pos == i_size_read(inode). And we need to take into account this code [1]:
> > 
> > 	if (iblock >= hip->fs_blocks) {
> > 		if (!create)
> > 			return 0;
> > 		if (iblock > hip->fs_blocks) <-- This is the rejection of hole
> > 			return -EIO;
> > 		if (ablock >= hip->alloc_blocks) {
> > 			res = hfsplus_file_extend(inode, false);
> > 			if (res)
> > 				return res;
> > 		}
> > 	}
> > 
> > The generic_write_end() changes the inode size: i_size_write(inode, pos +
> > copied).
> 
> I think that it's not problem.
> 
> hfsplus_write_begin()
>   cont_write_begin()
>     cont_expand_zero()
> 
> cont_expand_zero() calls hfsplus_get_block() to allocate blocks between
> i_size_read(inode) and pos, if pos > i_size_read(inode).
> 

Currently, HFS/HFS+ expect that file should be extended without holes. It means
that next allocating block should be equal to number of allocated blocks in
file. If pos > i_size_read(inode), then it means that next allocating block is
not equal to number of allocated blocks in file.

If you imply that requested length could include multiple blocks for allocation,
then next allocating block should be equal to number of allocated blocks on
every step. And if the next allocating block is bigger than number of allocated
blocks in file, then hole creation is requested.

So, what are we discussing here? :)

> > 
> > > wouldn't the condition "pos - i_size_read(inode) > free space" be better?
> > > Also instead of checking every time in hfsplus_write_begin() or
> > > hfsplus_file_extend(), how about implementing the check in the
> > > file_operations->write_iter callback function, and returing EFBIG?
> > 
> > Which callback do you mean here? I am not sure that it's good idea.
> > 
> 
> Here is a simple code snippet.
> 
>  static const struct file_operations hfsplus_file_operations = {
> ...
> -       .write_iter     = generic_file_write_iter,
> +       .write_iter     = hfsplus_file_write_iter,
> ...
> 
> +ssize_t hfsplus_file_write_iter(struct kiocb *iocb, struct iov_iter *iter)
> +{
> ...
> +       // check iocb->ki_pos is beyond i_size
> +
> +       ret = generic_file_write_iter(iocb, iter);
> 

The hfsplus_write_begin() will be called before hfsplus_file_write_iter() if we
are trying to extend the file. And hfsplus_get_block() calls
hfsplus_file_extend() that will call hfsplus_block_allocate(). So, everything
will happen before hfsplus_file_write_iter() call. What's the point to have the
check here?

Thanks,
Slava.

> 

Re: [PATCH] hfsplus: limit sb_maxbytes to partition size

[Hyunchul Lee]

On Fri, Mar 06, 2026 at 08:08:40PM +0000, Viacheslav Dubeyko wrote:
> On Fri, 2026-03-06 at 11:05 +0900, Hyunchul Lee wrote:
> > On Fri, Mar 06, 2026 at 01:23:16AM +0000, Viacheslav Dubeyko wrote:
> > > On Fri, 2026-03-06 at 09:57 +0900, Hyunchul Lee wrote:
> > > > On Thu, Mar 05, 2026 at 11:21:19PM +0000, Viacheslav Dubeyko wrote:
> > > > > On Thu, 2026-03-05 at 10:52 +0900, Hyunchul Lee wrote:
> > > > > > > > 
> > > > > > > > Sorry it's generic/285, not generic/268.
> > > > > > > > in generic/285, there is a test that creates a hole exceeding the block
> > > > > > > > size and appends small data to the file. hfsplus fails because it fills
> > > > > > > > the block device and returns ENOSPC. However if it returns EFBIG
> > > > > > > > instead, the test is skipped.
> > > > > > > > 
> > > > > > > > For writes like xfs_io -c "pwrite 8t 512", should fops->write_iter
> > > > > > > > returns ENOSPC, or would it be better to return EFBIG?
> > > > > > > > > 
> > > > > > > 
> > > > > > > Current hfsplus_file_extend() implementation doesn't support holes. I assume you
> > > > > > > mean this code [1]:
> > > > > > > 
> > > > > > >         len = hip->clump_blocks;
> > > > > > >         start = hfsplus_block_allocate(sb, sbi->total_blocks, goal, &len);
> > > > > > >         if (start >= sbi->total_blocks) {
> > > > > > >                 start = hfsplus_block_allocate(sb, goal, 0, &len);
> > > > > > >                 if (start >= goal) {
> > > > > > >                         res = -ENOSPC;
> > > > > > >                         goto out;
> > > > > > >                 }
> > > > > > >         }
> > > > > > > 
> > > > > > > Am I correct?
> > > > > > > 
> > > > > > Yes,
> > > > > > 
> > > > > > hfsplus_write_begin()
> > > > > >   cont_write_begin()
> > > > > >     cont_expand_zero()
> > > > > > 
> > > > > > 1) xfs_io -c "pwrite 8t 512"
> > > > > > 2) hfsplus_begin_write() is called with offset 2^43 and length 512
> > > > > > 3) cont_expand_zero() allocates and zeroes out one block repeatedly
> > > > > > for the range
> > > > > > 0 to 2^43 - 1. To achieve this, hfsplus_write_begin() is called repeatedly.
> > > > > > 4) hfsplus_write_begin() allocates one block through hfsplus_get_block() =>
> > > > > > hfsplus_file_extend()
> > > > > 
> > > > > I think we can consider these directions:
> > > > > 
> > > > > (1) Currently, HFS+ code doesn't support holes. So, it means that
> > > > > hfsplus_write_begin() can check pos variable and i_size_read(inode). If pos is
> > > > > bigger than i_size_read(inode), then hfsplus_file_extend() will reject such
> > > > > request. So, we can return error code (probably, -EFBIG) for this case without
> > > > > calling hfsplus_file_extend(). But, from another point of view, maybe,
> > > > > hfsplus_file_extend() could be one place for this check. Does it make sense?
> > > > > 
> > > > > (2) I think that hfsplus_file_extend() could treat hole or absence of free
> > > > > blocks like -ENOSPC. Probably, we can change the error code from -ENOSPC to -
> > > > > EFBIG in hfsplus_write_begin(). What do you think?
> > > > > 
> > > > Even if holes are not supported, shouldn't the following writes be
> > > > supported?
> > > > 
> > > > xfs_io -f -c "pwrite 4k 512" <file-path>
> > > > 
> > > > If so, since we need to support cases where pos > i_size_read(inode),
> > > 
> > > The pos > i_size_read(inode) means that you create the hole. Because,
> > 
> > That's correct. However I believe that not supporting writes like the
> > one mentioned above is a significant limitation. Filesystems that don't
> > support sparse files, such as exFAT, allocate blocks and fill them with
> > zeros.
> > 
> 
> You are welcomed to write the code for HFS/HFS+. :) I'll be happy to see such
> support.
> 
> > > oppositely, when HFS+ logic tries to allocate new block, then it expects to have
> > > pos == i_size_read(inode). And we need to take into account this code [1]:
> > > 
> > > 	if (iblock >= hip->fs_blocks) {
> > > 		if (!create)
> > > 			return 0;
> > > 		if (iblock > hip->fs_blocks) <-- This is the rejection of hole
> > > 			return -EIO;
> > > 		if (ablock >= hip->alloc_blocks) {
> > > 			res = hfsplus_file_extend(inode, false);
> > > 			if (res)
> > > 				return res;
> > > 		}
> > > 	}
> > > 
> > > The generic_write_end() changes the inode size: i_size_write(inode, pos +
> > > copied).
> > 
> > I think that it's not problem.
> > 
> > hfsplus_write_begin()
> >   cont_write_begin()
> >     cont_expand_zero()
> > 
> > cont_expand_zero() calls hfsplus_get_block() to allocate blocks between
> > i_size_read(inode) and pos, if pos > i_size_read(inode).
> > 
> 
> Currently, HFS/HFS+ expect that file should be extended without holes. It means
> that next allocating block should be equal to number of allocated blocks in
> file. If pos > i_size_read(inode), then it means that next allocating block is
> not equal to number of allocated blocks in file.
> 
> If you imply that requested length could include multiple blocks for allocation,
> then next allocating block should be equal to number of allocated blocks on
> every step. And if the next allocating block is bigger than number of allocated
> blocks in file, then hole creation is requested.
> 
> So, what are we discussing here? :)
> 

The email is getting lengty, so I will try to summarize the
discussion. :)

As mentioned before, if hfsplus_write_begin() returns an error when
pos > i_size_read(inode), the following write will fail:

xfs_io -f -c "pwrite 4k 512" <file-path>

However, currently hfsplus allows this write.

Therefore, the condition, pos - i_size_read(inode) > free space is
necessary, and futhermore, I think it is better to check the condtion
in write_iter() instead of write_begin().


> > > 
> > > > wouldn't the condition "pos - i_size_read(inode) > free space" be better?
> > > > Also instead of checking every time in hfsplus_write_begin() or
> > > > hfsplus_file_extend(), how about implementing the check in the
> > > > file_operations->write_iter callback function, and returing EFBIG?
> > > 
> > > Which callback do you mean here? I am not sure that it's good idea.
> > > 
> > 
> > Here is a simple code snippet.
> > 
> >  static const struct file_operations hfsplus_file_operations = {
> > ...
> > -       .write_iter     = generic_file_write_iter,
> > +       .write_iter     = hfsplus_file_write_iter,
> > ...
> > 
> > +ssize_t hfsplus_file_write_iter(struct kiocb *iocb, struct iov_iter *iter)
> > +{
> > ...
> > +       // check iocb->ki_pos is beyond i_size
> > +
> > +       ret = generic_file_write_iter(iocb, iter);
> > 
> 
> The hfsplus_write_begin() will be called before hfsplus_file_write_iter() if we
> are trying to extend the file. And hfsplus_get_block() calls
> hfsplus_file_extend() that will call hfsplus_block_allocate(). So, everything
> will happen before hfsplus_file_write_iter() call. What's the point to have the
> check here?

The callstack for write(2) is as follows. Am I misunderstanding
something?

write()
  do_sync_write()
    fops->write_iter()
    generic_file_write_iter()
      aops->write_begin()
      hfsplus_write_begin()

> 
> Thanks,
> Slava.
> 
> > 

-- 
Thanks,
Hyunchul

RE: [PATCH] hfsplus: limit sb_maxbytes to partition size

[Viacheslav Dubeyko]

On Mon, 2026-03-09 at 09:52 +0900, Hyunchul Lee wrote:
> 
<skipped>

> 
> The email is getting lengty, so I will try to summarize the
> discussion. :)
> 

I think I would like to see the second version of the patch. It will be easier
to review and discuss your solution in the form of patch. Otherwise, our
discussion goes nowhere now. :)

Thanks,
Slava.

Re: [PATCH] hfsplus: limit sb_maxbytes to partition size

[Hyunchul Lee]

On Mon, Mar 09, 2026 at 07:47:53PM +0000, Viacheslav Dubeyko wrote:
> On Mon, 2026-03-09 at 09:52 +0900, Hyunchul Lee wrote:
> > 
> <skipped>
> 
> > 
> > The email is getting lengty, so I will try to summarize the
> > discussion. :)
> > 
> 
> I think I would like to see the second version of the patch. It will be easier
> to review and discuss your solution in the form of patch. Otherwise, our
> discussion goes nowhere now. :)
> 

Okay, I will prepare the patch and send it out. 

> Thanks,
> Slava.

-- 
Thanks,
Hyunchul