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

Hyunchul Lee
2026-03-06T00:57:54Z

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