Recent changes mean sb_min_blocksize() can now fail

Recent changes mean sb_min_blocksize() can now fail

[Phillip Lougher]

Hi,

A recent (post 6.14) change to the kernel means sb_min_blocksize() can now fail,
and any filesystem which doesn't check the result may behave unexpectedly as a
result.  This change has recently affected Squashfs, and checking the kernel code,
a number of other filesystems including isofs, gfs2, exfat, fat and xfs do not
check the result.  This is a courtesy email to warn others of this change.

The following emails give the relevant details.

https://lore.kernel.org/all/2a13ea1c-08df-4807-83d4-241831b7a2ec@squashfs.org.uk/
https://lore.kernel.org/all/129d4f39-6922-44e9-8b1c-6455ee564dda@squashfs.org.uk/

Regards

Phillip

Re: Recent changes mean sb_min_blocksize() can now fail

[Jan Kara]

Hi!

On Tue 08-04-25 06:33:53, Phillip Lougher wrote:
> A recent (post 6.14) change to the kernel means sb_min_blocksize() can now fail,
> and any filesystem which doesn't check the result may behave unexpectedly as a
> result.  This change has recently affected Squashfs, and checking the kernel code,
> a number of other filesystems including isofs, gfs2, exfat, fat and xfs do not
> check the result.  This is a courtesy email to warn others of this change.
> 
> The following emails give the relevant details.
> 
> https://lore.kernel.org/all/2a13ea1c-08df-4807-83d4-241831b7a2ec@squashfs.org.uk/
> https://lore.kernel.org/all/129d4f39-6922-44e9-8b1c-6455ee564dda@squashfs.org.uk/

Indeed. Thanks for the heads up!

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: Recent changes mean sb_min_blocksize() can now fail

[Jan Kara]

On Tue 08-04-25 12:39:52, Jan Kara wrote:
> Hi!
> 
> On Tue 08-04-25 06:33:53, Phillip Lougher wrote:
> > A recent (post 6.14) change to the kernel means sb_min_blocksize() can now fail,
> > and any filesystem which doesn't check the result may behave unexpectedly as a
> > result.  This change has recently affected Squashfs, and checking the kernel code,
> > a number of other filesystems including isofs, gfs2, exfat, fat and xfs do not
> > check the result.  This is a courtesy email to warn others of this change.
> > 
> > The following emails give the relevant details.
> > 
> > https://lore.kernel.org/all/2a13ea1c-08df-4807-83d4-241831b7a2ec@squashfs.org.uk/
> > https://lore.kernel.org/all/129d4f39-6922-44e9-8b1c-6455ee564dda@squashfs.org.uk/
> 
> Indeed. Thanks for the heads up!

But isofs is actually fine since setting bdev block size needs exclusive open
(i.e., has to happen before filesystem mount begins and claims bdev) and
isofs does:

	if (bdev_logical_block_size(s->s_bdev) > 2048)
		bail

in its isofs_fill_super().

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: Recent changes mean sb_min_blocksize() can now fail

[OGAWA Hirofumi]

Phillip Lougher <phillip@squashfs.org.uk> writes:

> A recent (post 6.14) change to the kernel means sb_min_blocksize() can now fail,
> and any filesystem which doesn't check the result may behave unexpectedly as a
> result.  This change has recently affected Squashfs, and checking the kernel code,
> a number of other filesystems including isofs, gfs2, exfat, fat and xfs do not
> check the result.  This is a courtesy email to warn others of this change.
>
> The following emails give the relevant details.
>
> https://lore.kernel.org/all/2a13ea1c-08df-4807-83d4-241831b7a2ec@squashfs.org.uk/
> https://lore.kernel.org/all/129d4f39-6922-44e9-8b1c-6455ee564dda@squashfs.org.uk/

Thanks. FATfs should have no issue.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

RE: Recent changes mean sb_min_blocksize() can now fail

[Sungjong Seo]

Hi, All

> Hi!
> 
> On Tue 08-04-25 06:33:53, Phillip Lougher wrote:
> > A recent (post 6.14) change to the kernel means sb_min_blocksize() can
> now fail,
> > and any filesystem which doesn't check the result may behave
> unexpectedly as a
> > result.  This change has recently affected Squashfs, and checking the
> kernel code,
> > a number of other filesystems including isofs, gfs2, exfat, fat and xfs
> do not
> > check the result.  This is a courtesy email to warn others of this
> change.
> >
> > The following emails give the relevant details.
> >
> > https://lore.kernel.org/all/2a13ea1c-08df-4807-83d4-
> 241831b7a2ec@squashfs.org.uk/
> > https://lore.kernel.org/all/129d4f39-6922-44e9-8b1c-
> 6455ee564dda@squashfs.org.uk/
> 
> Indeed. Thanks for the heads up!
> 
> 								Honza
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

exfat-fs calls it to set it to the minimum value of 512, but it's not
particularly problematic if it fails.

Thank you
B.R.
Sungjong Seo

Re: Recent changes mean sb_min_blocksize() can now fail

[Darrick J. Wong]

On Tue, Apr 08, 2025 at 06:33:53AM +0100, Phillip Lougher wrote:
> Hi,
> 
> A recent (post 6.14) change to the kernel means sb_min_blocksize() can now fail,
> and any filesystem which doesn't check the result may behave unexpectedly as a
> result.  This change has recently affected Squashfs, and checking the kernel code,
> a number of other filesystems including isofs, gfs2, exfat, fat and xfs do not
> check the result.  This is a courtesy email to warn others of this change.

Thanks for the heads up.  xfs always passes in BBSIZE (aka 512) and
doesn't use the bdev pagecache so I think it's unaffected by failures.

--D

> The following emails give the relevant details.
> 
> https://lore.kernel.org/all/2a13ea1c-08df-4807-83d4-241831b7a2ec@squashfs.org.uk/
> https://lore.kernel.org/all/129d4f39-6922-44e9-8b1c-6455ee564dda@squashfs.org.uk/
> 
> Regards
> 
> Phillip
> 

Re: Recent changes mean sb_min_blocksize() can now fail

[Luis Chamberlain]

On Tue, Apr 08, 2025 at 12:51:37PM +0200, Jan Kara wrote:
> On Tue 08-04-25 12:39:52, Jan Kara wrote:
> > Hi!
> > 
> > On Tue 08-04-25 06:33:53, Phillip Lougher wrote:
> > > A recent (post 6.14) change to the kernel means sb_min_blocksize() can now fail,
> > > and any filesystem which doesn't check the result may behave unexpectedly as a
> > > result.  This change has recently affected Squashfs, and checking the kernel code,
> > > a number of other filesystems including isofs, gfs2, exfat, fat and xfs do not
> > > check the result.  This is a courtesy email to warn others of this change.
> > > 
> > > The following emails give the relevant details.
> > > 
> > > https://lore.kernel.org/all/2a13ea1c-08df-4807-83d4-241831b7a2ec@squashfs.org.uk/
> > > https://lore.kernel.org/all/129d4f39-6922-44e9-8b1c-6455ee564dda@squashfs.org.uk/
> > 
> > Indeed. Thanks for the heads up!
> 
> But isofs is actually fine since setting bdev block size needs exclusive open
> (i.e., has to happen before filesystem mount begins and claims bdev) and
> isofs does:
> 
> 	if (bdev_logical_block_size(s->s_bdev) > 2048)
> 		bail
> 
> in its isofs_fill_super().

Regardless, we added commit a64e5a596067bddb ("bdev: add back PAGE_SIZE
block size validation for sb_set_blocksize()" to effectively revert back
to the original behaviour, and so only filesystems which have FS_LBS are
not blocked by PAGE_SIZE.

Let me know if you still are seeing issues even with this patch merged.

  Luis

Re: Recent changes mean sb_min_blocksize() can now fail

[Kent Overstreet]

On Tue, Apr 08, 2025 at 06:33:53AM +0100, Phillip Lougher wrote:
> Hi,
> 
> A recent (post 6.14) change to the kernel means sb_min_blocksize() can now fail,
> and any filesystem which doesn't check the result may behave unexpectedly as a
> result.  This change has recently affected Squashfs, and checking the kernel code,
> a number of other filesystems including isofs, gfs2, exfat, fat and xfs do not
> check the result.  This is a courtesy email to warn others of this change.
> 
> The following emails give the relevant details.
> 
> https://lore.kernel.org/all/2a13ea1c-08df-4807-83d4-241831b7a2ec@squashfs.org.uk/
> https://lore.kernel.org/all/129d4f39-6922-44e9-8b1c-6455ee564dda@squashfs.org.uk/
> 
> Regards
> 
> Phillip

this would be a good time to use __must_check