d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Two bug fixes to UFS/FFS superblock integrity checks when reading a superblock.

Browse Source 
Two bugs have been reported with the UFS/FFS superblock integrity
checks that were added in commit 076002f24d35.

The code checked that fs_sblockactualloc was properly set to the
location of the superblock. The fs_sblockactualloc field was an
addition to the superblock in commit dffce2150eea on Jan 26 2018
and used a field that was zero in filesystems created before it
was added. The integrity check had to be expanded to accept the
fs_sblockactualloc field being zero so as not to reject filesystems
created before Jan 26 2018.

The integrity check set an upper bound on the value of fs_maxcontig
based on the maximum transfer size supported by the kernel. It
required that fs->fs_maxcontig <= maxphys / fs->fs_bsize. The kernel
variable maxphys defines the maximum transfer size permitted by the
controllers and/or buffering. The fs_maxcontig parameter controls the
maximum number of blocks that the filesystem will read or write in
a single transfer. It is calculated when the filesystem is created
as maxphys / fs_bsize. The bug appeared in the loader because it
uses a maxphys of 128K even when running on a system that supports
larger values. If the filesystem was built on a system that supports
a larger maxphys (1M is typical) it will have configured fs_maxcontig
for that larger system so would fail the test when run with the smaller
maxphys used by the loader. So we bound the upper allowable limit
for fs_maxconfig to be able to at least work with a 1M maxphys on the
smallest block size filesystem: 1M / 4096 == 256. We then use the
limit for fs_maxcontig as fs_maxcontig <= MAX(256, maxphys / fs_bsize).
There is no harm in allowing the mounting of filesystems that make larger
than maxphys I/O requests because those (mostly 32-bit machines) can
(very slowly) handle I/O requests that exceed maxphys.

Thanks to everyone who helped sort out the problems and the fixes.

Reported by:  Cy Schubert, David Wolfskill
Diagnosis by: Mark Johnston, John Baldwin
Reviewed by:  Warner Losh
Tested by:    Cy Schubert, David Wolfskill
MFC after:    1 month (with 076002f24d35)
Differential Revision: https://reviews.freebsd.org/D35219
This commit is contained in:
Kirk McKusick 2022-05-31 19:55:54 -07:00
parent d18b4bec98
commit bc218d8920
1 changed files with 22 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
sys/ufs/ffs/ffs_subr.c
View File

@ -319,7 +319,8 @@ validate_sblock(struct fs *fs, int isaltsblk)
sectorsize = dbtob(1); sectorsize = dbtob(1);
if (fs->fs_magic == FS_UFS2_MAGIC) { if (fs->fs_magic == FS_UFS2_MAGIC) {
if ((!isaltsblk && (fs->fs_sblockloc != SBLOCK_UFS2 || if ((!isaltsblk && (fs->fs_sblockloc != SBLOCK_UFS2 ||
fs->fs_sblockactualloc != SBLOCK_UFS2)) || !(fs->fs_sblockactualloc == 0 ||
fs->fs_sblockactualloc == SBLOCK_UFS2))) ||
fs->fs_maxsymlinklen != ((UFS_NDADDR + UFS_NIADDR) * fs->fs_maxsymlinklen != ((UFS_NDADDR + UFS_NIADDR) *
sizeof(ufs2_daddr_t)) || sizeof(ufs2_daddr_t)) ||
fs->fs_nindir != fs->fs_bsize / sizeof(ufs2_daddr_t) || fs->fs_nindir != fs->fs_bsize / sizeof(ufs2_daddr_t) ||
@ -327,7 +328,8 @@ validate_sblock(struct fs *fs, int isaltsblk)
return (ENOENT); return (ENOENT);
} else if (fs->fs_magic == FS_UFS1_MAGIC) { } else if (fs->fs_magic == FS_UFS1_MAGIC) {
if ((!isaltsblk && (fs->fs_sblockloc > SBLOCK_UFS1 || if ((!isaltsblk && (fs->fs_sblockloc > SBLOCK_UFS1 ||
fs->fs_sblockactualloc != SBLOCK_UFS1)) || !(fs->fs_sblockactualloc == SBLOCK_UFS1 ||
fs->fs_sblockactualloc == 0))) ||
fs->fs_nindir != fs->fs_bsize / sizeof(ufs1_daddr_t) || fs->fs_nindir != fs->fs_bsize / sizeof(ufs1_daddr_t) ||
fs->fs_inopb != fs->fs_bsize / sizeof(struct ufs1_dinode) || fs->fs_inopb != fs->fs_bsize / sizeof(struct ufs1_dinode) ||
fs->fs_maxsymlinklen != ((UFS_NDADDR + UFS_NIADDR) * fs->fs_maxsymlinklen != ((UFS_NDADDR + UFS_NIADDR) *
@ -423,13 +425,26 @@ validate_sblock(struct fs *fs, int isaltsblk)
fs->fs_size > fs->fs_ncg * fs->fs_fpg) fs->fs_size > fs->fs_ncg * fs->fs_fpg)
return (ENOENT); return (ENOENT);
/* /*
* Maxcontig sets the default for the maximum number of blocks * With file system clustering it is possible to allocate
* that may be allocated sequentially. With file system clustering * many contiguous blocks. The kernel variable maxphys defines
* it is possible to allocate contiguous blocks up to the maximum * the maximum transfer size permitted by the controller and/or
* transfer size permitted by the controller or buffering. * buffering. The fs_maxcontig parameter controls the maximum
* number of blocks that the filesystem will read or write
* in a single transfer. It is calculated when the filesystem
* is created as maxphys / fs_bsize. The loader uses a maxphys
* of 128K even when running on a system that supports larger
* values. If the filesystem was built on a system that supports
* a larger maxphys (1M is typical) it will have configured
* fs_maxcontig for that larger system. So we bound the upper
* allowable limit for fs_maxconfig to be able to at least
* work with a 1M maxphys on the smallest block size filesystem:
* 1M / 4096 == 256. There is no harm in allowing the mounting of
* filesystems that make larger than maxphys I/O requests because
* those (mostly 32-bit machines) can (very slowly) handle I/O
* requests that exceed maxphys.
*/ */
if (fs->fs_maxcontig < 1 || if (fs->fs_maxcontig < 1 ||
fs->fs_maxcontig > MAX(1, maxphys / fs->fs_bsize)) fs->fs_maxcontig > MAX(256, maxphys / fs->fs_bsize))
return (ENOENT); return (ENOENT);
if (fs->fs_maxcontig < 0 || if (fs->fs_maxcontig < 0 ||
(fs->fs_maxcontig == 0 && fs->fs_contigsumsize != 0) || (fs->fs_maxcontig == 0 && fs->fs_contigsumsize != 0) ||