Commit Graph

1568 Commits

Author SHA1 Message Date
Kirk McKusick
ddf162d1d1 ufs: handle LoR between snap lock and vnode lock
When a filesystem is mounted all of its associated snapshots must
be activated. It first allocates a snapshot lock (snaplk) that will
be shared by all the snapshot vnodes associated with the filesystem.
As part of each snapshot file activation, it must replace its own
ufs vnode lock with the snaplk. In this way acquiring the snaplk
gives exclusive access to all the snapshots for the filesystem.

A write to a ufs vnode first acquires the ufs vnode lock for the
file to be written then acquires the snaplk. Once it has the snaplk,
it can check all the snapshots to see if any of them needs to make
a copy of the block that is about to be written. This ffs_copyonwrite()
code path establishes the ufs vnode followed by snaplk locking
order.

When a filesystem is unmounted it has to release all of its snapshot
vnodes. Part of doing the release is to revert the snapshot vnode
from using the snaplk to using its original vnode lock. While holding
the snaplk, the vnode lock has to be acquired, the vnode updated
to reference it, then the snaplk released. Acquiring the vnode lock
while holding the snaplk violates the ufs vnode then snaplk order.
Because the vnode lock is unused, using LK_EXCLUSIVE | LK_NOWAIT
to acquire it will always succeed and the LK_NOWAIT prevents the
reverse lock order from being recorded.

This change was made in January 2021 (173779b98f) to avoid an LOR
violation in ffs_snapshot_unmount(). The same LOR issue was recently
found again when removing a snapshot in ffs_snapremove() which must
also revert the snaplk to the original vnode lock as part of freeing it.

The unwind in ffs_snapremove() deals with the case in which the
snaplk is held as a recursive lock holding multiple references.
Specifically an equal number of references are made on the vnode
lock. This change factors out the lock reversion operations into a
new function revert_snaplock() which handles both the recursive
locks and avoids the LOR. The new revert_snaplock() function is
then used in both ffs_snapshot_unmount() and in ffs_snapremove().

Reviewed by:  kib
Tested by:    Peter Holm
MFC after:    2 weeks
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D33946
2022-01-27 23:03:35 -08:00
Kirk McKusick
7ef56fb049 Avoid unnecessary setting of UFS flag requesting fsck(8) be run.
When the kernel is requested to mount a filesystem with a bad superblock
check hash, it would set the flag in the superblock requesting that the
fsck(8) program be run. The flag is only written to disk as part of a
superblock update. Since the superblock always has its check hash updated
when it is written to disk, the problem for which the flag has been set
will no longer exist. Hence, it is counter-productive to set the flag
as it will just cause an unnecessary run of fsck if it ever gets written.

Sponsored by: Netflix
2022-01-09 16:18:28 -08:00
Kirk McKusick
1fbcaa13b0 When doing a read-only mount of a UFS filesystem using gjournal(8),
suppress error message about a missing gjournal provider.

Submitted by: Andreas Longwitz
MFC after:    2 weeks
Sponsored by: Netflix
2022-01-02 14:04:39 -08:00
Jessica Clarke
324150d6da ufs: Avoid subobject overflow in snapshot expunge code
The code here tries to be smart and zeroes out both di_db and di_ib with
a single bzero call, thereby overrunning the di_db subobject. This is
fine on most architectures, if a little dodgy. However, on CHERI, the
compiler can optionally restrict the bounds on pointers to subobjects to
just that subobject, in order to mitigate intra-object buffer overflows,
and this is enabled in CheriBSD's pure-capability kernels.

Instead, use separate bzero calls for each array, and let the compiler
optimise it as it sees fit; even if it's not generating inline zeroing
code, Clang will happily optimise two consecutive bzero's to a single
larger call.

Reviewed by:	mckusick
Differential Revision:	https://reviews.freebsd.org/D33651
2022-01-02 20:55:49 +00:00
Jessica Clarke
5b13fa7987 ufs: Rework shortlink handling to avoid subobject overflows
Shortlinks occupy the space of both di_db and di_ib when used. However,
everywhere that wants to read or write a shortlink takes a pointer do
di_db and promptly runs off the end of it into di_ib. This is fine on
most architectures, if a little dodgy. However, on CHERI, the compiler
can optionally restrict the bounds on pointers to subobjects to just
that subobject, in order to mitigate intra-object buffer overflows, and
this is enabled in CheriBSD's pure-capability kernels.

Instead, clean this up by inserting a union such that a new di_shortlink
can be added with the right size and element type, avoiding the need to
cast and allowing the use of the DIP macro to access the field. This
also mirrors how the ext2fs code implements extents support, with the
exact same structure other than having a uint32_t i_data[] instead of a
char di_shortlink[].

Reviewed by:	mckusick, jhb
Differential Revision:	https://reviews.freebsd.org/D33650
2022-01-02 20:55:36 +00:00
Gordon Bergling
f9af3151fa Revert "ffs(3): Fix a typo in a sysctl description"
It should be

- s/contigous/contiguous/ not continuous

Reported by:	tuexen@

This reverts commit 42efe994ec.
2021-12-05 13:45:47 +01:00
Gordon Bergling
42efe994ec ffs(3): Fix a typo in a sysctl description
- s/contigous/continuous/

MFC after:	3 days
2021-12-04 12:15:34 +01:00
Mateusz Guzik
7e1d3eefd4 vfs: remove the unused thread argument from NDINIT*
See b4a58fbf64 ("vfs: remove cn_thread")

Bump __FreeBSD_version to 1400043.
2021-11-25 22:50:42 +00:00
Gordon Bergling
bebff61587 ffs_softdep: Fix a typo in a source code comment
- s/conditonally/conditionally/

MFC after:	3 days
2021-11-19 19:17:41 +01:00
Konstantin Belousov
c34a5148e8 ffs: fix newly introduced LOR between mntfs vnode lock and topology lock
The mntfs vnode lock should be before topology, as established in
ffs_mountfs().  Extend the locked region in ffs_unmount().

Reported and reviewed by:	markj
Sponsored by:	The FreeBSD Foundation
MFC after:	1 week
Differential revision:	https://reviews.freebsd.org/D33013
2021-11-16 20:01:31 +02:00
Kirk McKusick
9b8eb1c5b6 Followup to f2b391528a to improve printed message.
Sponsored by: Netflix
2021-11-15 16:10:02 -08:00
Kirk McKusick
9e9dcac95a Allow forced r/w mount of UFS/FFS filesystem with a bad check hash.
Normally a UFS/FFS filesystem with a bad check hash can only be
mounted read only. With this commit the mount(8) -f (force) option
can be used to force a read-write mount of a UFS/FFS filesystem with
a bad check hash. Conveniently the filesystem will proceed to
update its on-disk superblock with a corrected check hash.

Sponsored by: Netflix
2021-11-15 16:03:47 -08:00
Kirk McKusick
f2b391528a Add ability to suppress UFS/FFS superblock check-hash failure messages.
When reading UFS/FFS superblocks that have check hashes, both the kernel
and libufs print an error message if the check hash is incorrect. This
commit adds the ability to request that the error message not be made.
It is intended for use by programs like fsck that wants to print its
own error message and by kernel subsystems like glabel that just wants
to check for possible filesystem types.

This capability will be used in followup commits.

Sponsored by: Netflix
2021-11-15 09:11:54 -08:00
Kirk McKusick
b366ee4868 Consolodate four copies of the STDSB define into a single place.
The STDSB macro is passed to the ffs_sbget() routine to fetch a
UFS/FFS superblock "from the stadard place". It was identically defined
in lib/libufs/libufs.h, stand/libsa/ufs.c, sys/ufs/ffs/ffs_extern.h,
and sys/ufs/ffs/ffs_subr.c. Delete it from these four files and
define it instead in sys/ufs/ffs/fs.h. All existing uses of this macro
already include sys/ufs/ffs/fs.h so no include changes need to be made.

No functional change intended.

Sponsored by: Netflix
2021-11-14 22:10:16 -08:00
Konstantin Belousov
eede22d66d ffs_snapshot: do not assert that um_devvp is locked
It is not, and the lock is not needed there

Reported and tested by:	pho
Reviewed by:	markj
Sponsored by:	The FreeBSD Foundation
MFC after:	1 week
Differential revision:	https://reviews.freebsd.org/D32761
2021-11-13 01:00:54 +02:00
Konstantin Belousov
25809a018d mntfs: lock mntfs pseudo devfs vnode properly
Require devvp locked for mntfs_freevp(), to have it locked around
vgone().  Make that true for ffs, which is the only consumer of
the interface.

Reported and tested by:	pho
Reviewed by:	markj
Sponsored by:	The FreeBSD Foundation
MFC after:	1 week
Differential revision:	https://reviews.freebsd.org/D32761
2021-11-13 01:00:41 +02:00
Konstantin Belousov
76b05e3e39 ffs: Remove assertions about locked um_devvp in several places
Namely, ffs_blkfree_cg(), and ffs_flushfiles().

Reported and tested by:	pho
Reviewed by:	markj
Sponsored by:	The FreeBSD Foundation
MFC after:	1 week
Differential revision:	https://reviews.freebsd.org/D32761
2021-11-13 01:00:33 +02:00
Konstantin Belousov
2030ee0e1b ufs: remove write-only variables
Mark variables as __diagused for invariant-only vars

Reviewed by:	imp, mjg
Sponsored by:	The FreeBSD Foundation
MFC after:	1 week
Differential revision:	https://reviews.freebsd.org/D32577
2021-10-21 21:40:46 +03:00
Mateusz Guzik
b4a58fbf64 vfs: remove cn_thread
It is always curthread.

Reviewed by:	kib
Differential Revision:	https://reviews.freebsd.org/D32453
2021-10-11 13:21:47 +00:00
Robert Wing
9acea16404 ffs: retire unused fsckpid mount option
The fsckpid mount option was introduced in 927a12ae16 along with a
couple sysctl's to support SU+J with snapshots. However, those sysctl's
were never used and eventually removed in f2620e9ceb.

There are no in-tree consumers of this mount option.

Reviewed by:	mckusick, kib
Differential Revision:	https://reviews.freebsd.org/D32015
2021-10-02 15:11:40 -08:00
Kirk McKusick
4a365e863f Avoid "consumer not attached in g_io_request" panic when disk lost
while using a UFS snapshot.

The UFS filesystem supports snapshots. Each snapshot is a file whose
contents are a frozen image of the disk partition on which the filesystem
resides. Each time an existing block in the filesystem is modified,
the filesystem checks whether that block was in use at the time that
the snapshot was taken. If so, and if it has not already been copied,
a new block is allocated from among the blocks that were not in use
at the time that the snapshot was taken and placed in the snapshot file
to replace the entry that has not yet been copied. The previous contents
of the block are copied to the newly allocated snapshot file block,
and the write to the original is then allowed to proceed.

The block allocation is done using the usual UFS_BALLOC() routine
which allocates the needed block in the snapshot and returns a
buffer that is set up to write data into the newly allocated block.
In usual filesystem operation, the contents for the new block is
copied from user space into the buffer and the buffer is then written
to the file using bwrite(), bawrite(), or bdwrite(). In the case of a
snapshot the new block must be filled from the disk block that is about
to be rewritten. The snapshot routine has a function readblock() that
it uses to read the `about to be rewritten' disk block.

/*
 * Read the specified block into the given buffer.
 */
static int
readblock(snapvp, bp, lbn)
	struct vnode *snapvp;
	struct buf *bp;
	ufs2_daddr_t lbn;
{
	struct inode *ip;
	struct bio *bip;
	struct fs *fs;

	ip = VTOI(snapvp);
	fs = ITOFS(ip);

	bip = g_alloc_bio();
	bip->bio_cmd = BIO_READ;
	bip->bio_offset = dbtob(fsbtodb(fs, blkstofrags(fs, lbn)));
	bip->bio_data = bp->b_data;
	bip->bio_length = bp->b_bcount;
	bip->bio_done = NULL;

	g_io_request(bip, ITODEVVP(ip)->v_bufobj.bo_private);
	bp->b_error = biowait(bip, "snaprdb");
	g_destroy_bio(bip);
	return (bp->b_error);
}

When the underlying disk fails, its GEOM module is removed.
Subsequent attempts to access it should return the ENXIO error.
The functionality of checking for the lost disk and returning
ENXIO is handled by the g_vfs_strategy() routine:

void
g_vfs_strategy(struct bufobj *bo, struct buf *bp)
{
	struct g_vfs_softc *sc;
	struct g_consumer *cp;
	struct bio *bip;

	cp = bo->bo_private;
	sc = cp->geom->softc;

	/*
	 * If the provider has orphaned us, just return ENXIO.
	 */
	mtx_lock(&sc->sc_mtx);
	if (sc->sc_orphaned || sc->sc_enxio_active) {
		mtx_unlock(&sc->sc_mtx);
		bp->b_error = ENXIO;
		bp->b_ioflags |= BIO_ERROR;
		bufdone(bp);
		return;
	}
	sc->sc_active++;
	mtx_unlock(&sc->sc_mtx);

	bip = g_alloc_bio();
	bip->bio_cmd = bp->b_iocmd;
	bip->bio_offset = bp->b_iooffset;
	bip->bio_length = bp->b_bcount;
	bdata2bio(bp, bip);
	if ((bp->b_flags & B_BARRIER) != 0) {
		bip->bio_flags |= BIO_ORDERED;
		bp->b_flags &= ~B_BARRIER;
	}
	if (bp->b_iocmd == BIO_SPEEDUP)
		bip->bio_flags |= bp->b_ioflags;
	bip->bio_done = g_vfs_done;
	bip->bio_caller2 = bp;
	g_io_request(bip, cp);
}

Only after checking that the device is present does it construct
the "bio" request and call g_io_request(). When readblock()
constructs its own "bio" request and calls g_io_request() directly
it panics with "consumer not attached in g_io_request" when the
underlying device no longer exists.

The fix is to have readblock() call g_vfs_strategy() rather than
constructing its own "bio" request:

/*
 * Read the specified block into the given buffer.
 */
static int
readblock(snapvp, bp, lbn)
	struct vnode *snapvp;
	struct buf *bp;
	ufs2_daddr_t lbn;
{
	struct inode *ip;
	struct fs *fs;

	ip = VTOI(snapvp);
	fs = ITOFS(ip);

	bp->b_iocmd = BIO_READ;
	bp->b_iooffset = dbtob(fsbtodb(fs, blkstofrags(fs, lbn)));
	bp->b_iodone = bdone;
	g_vfs_strategy(&ITODEVVP(ip)->v_bufobj, bp);
	bufwait(bp);
	return (bp->b_error);
}

Here it uses the buffer that will eventually be written to the disk.
The g_vfs_strategy() routine uses four parts of the buffer: b_bcount,
b_iocmd, b_iooffset, and b_data.

The b_bcount field is already correctly set for the buffer. It is
safe to set the b_iocmd and b_iooffset fields as they are set
correctly when the later write is done. The write path will also
clear the B_DONE flag that our use of the buffer will set. The
b_iodone callback has to be set to bdone() which will do just
notification that the I/O is done in bufdone(). The rest of
bufdone() includes things like processing the softdeps associated
with the buffer should not be done until the buffer has been
written. Bufdone() will set b_iodone back to NULL after using it,
so the full bufdone() processing will be done when the buffer is
written. The final change from the previous version of readblock()
is that it used the b_data for the destination of the read while
g_vfs_strategy() uses the bdata2bio() function to take advantage
of VMIO when it is available.

Differential revision: https://reviews.freebsd.org/D32150
Reviewed by:  kib, chs
MFC after:    1 week
Sponsored by: Netflix
2021-09-27 20:04:51 -07:00
Kirk McKusick
d7770a5495 Eliminate snaplk / bufwait LOR when creating UFS snapshots
Each vnode has an embedded lock that controls access to its contents.
However vnodes describing a UFS snapshot all share a single snapshot
lock to coordinate their access and update. As part of creating a
new UFS snapshot, it has to have its individual vnode lock replaced
with the filesystem's snapshot lock.

The lock order for regular vnodes with respect to buffer locks is that
they must first acquire the vnode lock, then a buffer lock. The order
for the snapshot lock is reversed: a buffer lock must be acquired before
the snapshot lock.

When creating a new snapshot, the snapshot file must retain its vnode
lock until it has allocated all the blocks that it needs before
switching to the snapshot lock. This update moves one final piece of
the initial snapshot block allocation so that it is done before the
newly created snapshot is switched to use the snapshot lock.

Reported by:  Witness code
MFC after:    1 week
Sponsored by: Netflix
2021-09-18 17:02:30 -07:00
Konstantin Belousov
197a4f29f3 buffer pager: allow get_blksize method to return error
Reported and reviewed by:	asomers
Sponsored by:	The FreeBSD Foundation
MFC after:	1 week
Differential revision:	https://reviews.freebsd.org/D31998
2021-09-17 20:29:55 +03:00
Robert Wing
440320b620 ffs: remove unused thread argument from ffs_reload()
MFC After:      1 week
Reviewed by:	imp, kib
Differential Revision:	https://reviews.freebsd.org/D31127
2021-09-04 12:25:10 -08:00
Konstantin Belousov
bb536de6c0 ffs_update(): Do not assume that EBUSY can only come LK_NOWAIT trylock
Instead do protective check for the local flags and do not interpret
EBUSY specially if we did not request trylock mode for bread().

Reviewed by:	mckusick
Reported and tested by:	pho
Sponsored by:	The FreeBSD Foundation
MFC after:	1 week
2021-08-31 07:38:35 +03:00
Konstantin Belousov
f822d4feb8 ffs_update(): recalculate flags after relocking the vnode
Inode type could migrate between snapshot and regular types while the
vnode is unlocked.  Recalculate flags specific for snapshot after relock.

Reviewed by:	mckusick
Reported and tested by:	pho
Sponsored by:	The FreeBSD Foundation
MFC after:	1 week
2021-08-31 07:38:35 +03:00
Keith Owens
3b29c8b4bd ddb: do not assume that ffs is mounted with softdep
Avoid a panic when debugging with "show ffs" in ddb.

Reviewed By:	kib, markj, mckusick
MFC after:	1 week
Sponsored by:	Dell EMC Isilon
Differential Revision: https://reviews.freebsd.org/D31622
2021-08-24 21:00:19 -05:00
Konstantin Belousov
8df4bc48c8 ufs rename: ensure that the result of ufs_checkpath() is stable
ufs_rename() calls ufs_checkpath() to ensure that the target directory
is not a child of the source.  If not, rename would create a loop.
For instance:
	source->X1->X2->target
and if source moved under target, we get corrupted filesystem.
Suppose that we initially have
	source->X1 .... and X2->target
where X1 is not on path from root to X2.  Then ufs_checkpath() accepts
the inodes, but there is nothing preventing parallel rename of X2 to become
under X1, after checkpath finished.

Ensure stability of ufs_checkpath() result by taking a per-mount sx in
ufs_rename right before ufs_checkpath() and till the end.

Reviewed by:	chs, mckusick
Tested by:	pho
Sponsored by:	The FreeBSD Foundation
MFC after:	2 weeks
2021-08-13 17:52:26 +03:00
Kirk McKusick
a91716efeb Clean up orphaned indirdep dependency structures after disk failure.
During forcible unmount after a disk failure there is a bug that
causes one or more indirdep dependency structures to fail to be
deallocated. Until we manage to track down why they fail to get
cleaned up, this code tracks them down and eliminates them so that
the unmount can succeed.

Reported by:  Peter Holm
Help from:    kib
Reviewed by:  Chuck Silvers
Tested by:    Peter Holm
MFC after:    7 days
Sponsored by: Netflix
2021-07-29 16:31:16 -07:00
Kirk McKusick
412b5e40a7 Diagnotic improvement to soft dependency structure management.
The soft updates diagnotic code keeps a list for each type of soft
update dependency. When a new block is allocated for a file it is
initially tracked by a "newblk" dependency. The "newblk" dependency
eventually becomes either an "allocdirect" dependency or an "indiralloc"
dependency. The diagnotic code failed to move the "newblk" from the list
of "newblk"s to its new type list.

No functional change intended.

Reviewed by:  Chuck Silvers (as part of a larger change)
Tested by:    Peter Holm (as part of a larger change)
Sponsored by: Netflix
2021-07-29 16:13:54 -07:00
Jason A. Harmening
211ec9b7d6 FFS: remove ffs_fsfail_task
Now that dounmount() supports a dedicated taskqueue, we can simply call
it with MNT_DEFERRED directly from the failing context.  This also
avoids blocking taskqueue_thread with a potentially-expensive unmount
operation.

Reviewed by:	kib, mckusick
Tested by:	pho
Differential Revision:	https://reviews.freebsd.org/D31016
2021-07-24 12:52:41 -07:00
Jason A. Harmening
c746ed724d Allow stacked filesystems to be recursively unmounted
In certain emergency cases such as media failure or removal, UFS will
initiate a forced unmount in order to prevent dirty buffers from
accumulating against the no-longer-usable filesystem.  The presence
of a stacked filesystem such as nullfs or unionfs above the UFS mount
will prevent this forced unmount from succeeding.

This change addreses the situation by allowing stacked filesystems to
be recursively unmounted on a taskqueue thread when the MNT_RECURSE
flag is specified to dounmount().  This call will block until all upper
mounts have been removed unless the caller specifies the MNT_DEFERRED
flag to indicate the base filesystem should also be unmounted from the
taskqueue.

To achieve this, the recently-added vfs_pin_from_vp()/vfs_unpin() KPIs
have been combined with the existing 'mnt_uppers' list used by nullfs
and renamed to vfs_register_upper_from_vp()/vfs_unregister_upper().
The format of the mnt_uppers list has also been changed to accommodate
filesystems such as unionfs in which a given mount may be stacked atop
more than one lower mount.  Additionally, management of lower FS
reclaim/unlink notifications has been split into a separate list
managed by a separate set of KPIs, as registration of an upper FS no
longer implies interest in these notifications.

Reviewed by:	kib, mckusick
Tested by:	pho
Differential Revision:	https://reviews.freebsd.org/D31016
2021-07-24 12:52:00 -07:00
John Baldwin
58109a87d4 Use an ANSI C function declaration for journal_check_space.
GCC6 fails to compile this due to a -Wstrict-prototypes error.

Sponsored by:	Chelsio Communications
2021-07-23 15:59:11 -07:00
Konstantin Belousov
50acaaef54 ffs_softdep: force sync if journal is low in journal_check_space
This effectively causes syncing of the mount point from softdep_prealloc(),
softdep_prerename(), and softdep_prelink().  Typically it avoids the need
for journal suspension at this point, at all.

Suggested and reviewed by:	mckusick
Discussed with:	markj
Tested by:	pho
Sponsored by:	The FreeBSD Foundation
MFC after:	2 weeks
Differential revision:	https://reviews.freebsd.org/D30041
2021-06-23 23:47:05 +03:00
Konstantin Belousov
2126f103e0 ffs_softdep.c: add journal_check_space() helper
Reviewed by:	mckusick
Discussed with:	markj
Tested by:	pho
Sponsored by:	The FreeBSD Foundation
MFC after:	2 weeks
Differential revision:	https://reviews.freebsd.org/D30041
2021-06-23 23:47:05 +03:00
Konstantin Belousov
64b494a105 softdep_prelink(): only do sync if other thread changed the vnode metadata since previous prelink
We call into softdep_prerename() and softdep_prelink() when there is
low free space in the journal. Functions sync all vnodes participating
in the VOP, in the hope that this would reduce journal utilization. But
if the vnodes are already synced, doing sync would only spend writes,
journal is filled not due to the records from modifications of our
vnodes.

Remember original seqc numbers for vnodes, and only initiate syncs when
seqc changed.

Reviewed by:	mckusick
Discussed with:	markj
Tested by:	pho
Sponsored by:	The FreeBSD Foundation
MFC after:	2 weeks
Differential revision:	https://reviews.freebsd.org/D30041
2021-06-23 23:46:54 +03:00
Konstantin Belousov
d4d289cd51 ffs: mark block (re-)allocations as seqc writes
Reviewed by:	mckusick
Discussed with:	markj
Tested by:	pho
Sponsored by:	The FreeBSD Foundation
MFC after:	2 weeks
Differential revision:	https://reviews.freebsd.org/D30041
2021-06-23 23:46:15 +03:00
Konstantin Belousov
d0929a990c ffs: reduce number of dvp relocks in softdep_prelink()
If vp == NULL, we unlocked and then immediately relocked dvp there.

Reviewed by:	mckusick
Discussed with:	markj
Tested by:	pho
Sponsored by:	The FreeBSD Foundation
MFC after:	2 weeks
Differential revision:	https://reviews.freebsd.org/D30041
2021-06-23 23:46:15 +03:00
Mark Johnston
b2f9575646 ffs: Correct the input size check in sysctl_ffs_fsck()
Make sure we return an error if no input was specified, since
SYSCTL_IN() will report success in that case.

Reported by:	KMSAN
Reviewed by:	mckusick
MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D30586
2021-05-31 18:59:18 -04:00
Konstantin Belousov
f784da883f Move mnt_maxsymlinklen into appropriate fs mount data structures
Reviewed by:	mckusick
Tested by:	pho
Sponsored by:	The FreeBSD Foundation
MFC after:	1 week
X-MFC-Note:	struct mount layout
Differential revision:	https://reviews.freebsd.org/D30325
2021-05-22 15:16:09 +03:00
Kirk McKusick
9a2fac6ba6 Fix handling of embedded symbolic links (and history lesson).
The original filesystem release (4.2BSD) had no embedded sysmlinks.
Historically symbolic links were just a different type of file, so
the content of the symbolic link was contained in a single disk block
fragment. We observed that most symbolic links were short enough that
they could fit in the area of the inode that normally holds the block
pointers. So we created embedded symlinks where the content of the
link was held in the inode's pointer area thus avoiding the need to
seek and read a data fragment and reducing the pressure on the block
cache. At the time we had only UFS1 with 32-bit block pointers,
so the test for a fastlink was:

	di_size < (NDADDR + NIADDR) * sizeof(daddr_t)

(where daddr_t would be ufs1_daddr_t today).

When embedded symlinks were added, a spare field in the superblock
with a known zero value became fs_maxsymlinklen. New filesystems
set this field to (NDADDR + NIADDR) * sizeof(daddr_t). Embedded
symlinks were assumed when di_size < fs->fs_maxsymlinklen. Thus
filesystems that preceeded this change always read from blocks
(since fs->fs_maxsymlinklen == 0) and newer ones used embedded
symlinks if they fit. Similarly symlinks created on pre-embedded
symlink filesystems always spill into blocks while newer ones will
embed if they fit.

At the same time that the embedded symbolic links were added, the
on-disk directory structure was changed splitting the former
u_int16_t d_namlen into u_int8_t d_type and u_int8_t d_namlen.
Thus fs_maxsymlinklen <= 0 (as used by the OFSFMT() macro) can
be used to distinguish old directory formats. In retrospect that
should have just been an added flag, but we did not realize we
needed to know about that change until it was already in production.

Code was split into ufs/ffs so that the log structured filesystem could
use ufs functionality while doing its own disk layout. This meant
that no ffs superblock fields could be used in the ufs code. Thus
ffs superblock fields that were needed in ufs code had to be copied
to fields in the mount structure. Since ufs_readlink needed to know
if a link was embedded, fs_maxlinklen gets copied to mnt_maxsymlinklen.

The kernel panic that arose to making this fix was triggered when a
disk error created an inode of type symlink with no allocated data
blocks but a large size. When readlink was called the uiomove was
attempted which segment faulted.

static int
ufs_readlink(ap)
	struct vop_readlink_args /* {
		struct vnode *a_vp;
		struct uio *a_uio;
		struct ucred *a_cred;
	} */ *ap;
{
	struct vnode *vp = ap->a_vp;
	struct inode *ip = VTOI(vp);
	doff_t isize;

	isize = ip->i_size;
	if ((isize < vp->v_mount->mnt_maxsymlinklen) ||
	    DIP(ip, i_blocks) == 0) { /* XXX - for old fastlink support */
		return (uiomove(SHORTLINK(ip), isize, ap->a_uio));
	}
	return (VOP_READ(vp, ap->a_uio, 0, ap->a_cred));
}

The second part of the "if" statement that adds

	DIP(ip, i_blocks) == 0) { /* XXX - for old fastlink support */

is problematic. It never appeared in BSD released by Berkeley because
as noted above mnt_maxsymlinklen is 0 for old format filesystems, so
will always fall through to the VOP_READ as it should. I had to dig
back through `git blame' to find that Rodney Grimes added it as
part of ``The big 4.4BSD Lite to FreeBSD 2.0.0 (Development) patch.''
He must have brought it across from an earlier FreeBSD. Unfortunately
the source-control logs for FreeBSD up to the merger with the
AT&T-blessed 4.4BSD-Lite conversion were destroyed as part of the
agreement to let FreeBSD remain unencumbered, so I cannot pin-point
where that line got added on the FreeBSD side.

The one change needed here is that mnt_maxsymlinklen is declared as
an `int' and should be changed to be `u_int64_t'.

This discovery led us to check out the code that deletes symbolic
links. Specifically

	if (vp->v_type == VLNK &&
	    (ip->i_size < vp->v_mount->mnt_maxsymlinklen ||
	     datablocks == 0)) {
		if (length != 0)
			panic("ffs_truncate: partial truncate of symlink");
		bzero(SHORTLINK(ip), (u_int)ip->i_size);
		ip->i_size = 0;
		DIP_SET(ip, i_size, 0);
		UFS_INODE_SET_FLAG(ip, IN_SIZEMOD | IN_CHANGE | IN_UPDATE);
		if (needextclean)
			goto extclean;
		return (ffs_update(vp, waitforupdate));
	}

Here too our broken symlink inode with no data blocks allocated
and a large size will segment fault as we are incorrectly using the
test that we have no data blocks to decide that it is an embdedded
symbolic link and attempting to bzero past the end of the inode.
The test for datablocks == 0 is unnecessary as the test for
ip->i_size < vp->v_mount->mnt_maxsymlinklen will do the right
thing in all cases.

The test for datablocks == 0 was added by David Greenman in this commit:

Author: David Greenman <dg@FreeBSD.org>
Date:   Tue Aug 2 13:51:05 1994 +0000

    Completed (hopefully) the kernel support for old style "fastlinks".

    Notes:
	svn path=/head/; revision=1821

I am guessing that he likely earlier added the incorrect test in the
ufs_readlink code.

I asked David if he had any recollection of why he made this change.
Amazingly, he still had a recollection of why he had made a one-line
change more than twenty years ago. And unsurpisingly it was because
he had been stuck between a rock and a hard place.

FreeBSD was up to 1.1.5 before the switch to the 4.4BSD-Lite code
base. Prior to that, there were three years of development in all
areas of the kernel, including the filesystem code, from the combined
set of people including Bill Jolitz, Patchkit contributors, and
FreeBSD Project members. The compatibility issue at hand was caused
by the FASTLINKS patches from Curt Mayer. In merging in the 4.4BSD-Lite
changes David had to find a way to provide compatibility with both
the changes that had been made in FreeBSD 1.1.5 and with 4.4BSD-Lite.
He felt that these changes would provide compatibility with both systems.

In his words:
``My recollection is that the 'FASTLINKS' symlinks support in
FreeBSD-1.x, as implemented by Curt Mayer, worked differently than
4.4BSD. He used a spare field in the inode to duplicately store the
length. When the 4.4BSD-Lite merge was done, the optimized symlinks
support for existing filesystems (those that were initialized in
FreeBSD-1.x) were broken due to the FFS on-disk structure of
4.4BSD-Lite differing from FreeBSD-1.x. My commit was needed to
restore the backward compatibility with FreeBSD-1.x filesystems.
I think it was the best that could be done in the somewhat urgent
circumstances of the post Berkeley-USL settlement. Also, regarding
Rod's massive commit with little explanation, some context: John
Dyson and I did the initial re-port of the 4.4BSD-Lite kernel to
the 386 platform in just 10 days. It was by far the most intense
hacking effort of my life. In addition to the porting of tons of
FreeBSD-1 code, I think we wrote more than 30,000 lines of new code
in that time to deal with the missing pieces and architectural
changes of 4.4BSD-Lite. We didn't make many notes along the way.
There was a lot of pressure to get something out to the rest of the
developer community as fast as possible, so detailed discrete commits
didn't happen - it all came as a giant wad, which is why Rod's
commit message was worded the way it was.''

Reported by:  Chuck Silvers
Tested by:    Chuck Silvers
History by:   David Greenman Lawrence
MFC after:    1 week
Sponsored by: Netflix
2021-05-16 17:04:11 -07:00
Konstantin Belousov
e3d6759585 b_vflags update requries bufobj lock
The trunc_dependencies() issue was reported by	Alexander Lochmann
<alexander.lochmann@tu-dortmund.de>, who found the problem by performing
lock analysis using LockDoc, see https://doi.org/10.1145/3302424.3303948.

Tested by:	pho
Sponsored by:	The FreeBSD Foundation
MFC after:	1 week
2021-04-15 15:47:42 +03:00
Konstantin Belousov
0b3948e73b softdep_unmount: assert that no dandling dependencies are left
Reviewed by:	mckusick
Tested by:	pho
Sponsored by:	The FreeBSD Foundation
MFC after:	2 weeks
Differential revision:	https://reviews.freebsd.org/D29178
2021-03-12 13:31:08 +02:00
Konstantin Belousov
7a8d4b4da6 FFS: assign fully initialized struct mount_softdeps to um_softdep
Other threads observing the non-NULL um_softdep can assume that it is
safe to use it. This is important for ro->rw remounts where change from
read-only to read-write status cannot be made atomic.

Reviewed by:	mckusick
Tested by:	pho
Sponsored by:	The FreeBSD Foundation
MFC after:	2 weeks
Differential revision:	https://reviews.freebsd.org/D29178
2021-03-12 13:31:08 +02:00
Konstantin Belousov
2af934cc15 Assert that um_softdep is NULL on free(ump), i.e. softdep_unmount() was called
Reviewed by:	mckusick
Tested by:	pho
Sponsored by:	The FreeBSD Foundation
MFC after:	2 weeks
Differential revision:	https://reviews.freebsd.org/D29178
2021-03-12 13:31:08 +02:00
Konstantin Belousov
f776c54cee ffs_mount: when remounting ro->rw and sbupdate failed, cleanup softdeps
Reviewed by:	mckusick
Tested by:	pho
Sponsored by:	The FreeBSD Foundation
MFC after:	2 weeks
Differential revision:	https://reviews.freebsd.org/D29178
2021-03-12 13:31:08 +02:00
Konstantin Belousov
d7e5e37416 softdep_unmount: handle spurious wakeups
Reviewed by:	mckusick
Tested by:	pho
Sponsored by:	The FreeBSD Foundation
MFC after:	2 weeks
Differential revision:	https://reviews.freebsd.org/D29178
2021-03-12 13:31:08 +02:00
Konstantin Belousov
fabbc3d879 softdep_flush(): do not access ump after we acked FLUSH_EXIT and unlocked SU lock
otherwise we might follow a pointer in the freed memory.

Reviewed by:	mckusick
Tested by:	pho
Sponsored by:	The FreeBSD Foundation
MFC after:	2 weeks
Differential revision:	https://reviews.freebsd.org/D29178
2021-03-12 13:31:08 +02:00
Konstantin Belousov
7c7a6681fa ffs: clear MNT_SOFTDEP earlier when remounting rw to ro
Suppose that we remount rw->ro and in parallel some reader tries to
instantiate a vnode, e.g. during lookup.  Suppose that softdep_unmount()
already started, but we did not cleared the MNT_SOFTDEP flag yet.
Then ffs_vgetf() calls into softdep_load_inodeblock() which accessed
destroyed hashes and freed memory.

Set/clear fs_ronly simultaneously (WRT to files flush) with MNT_SOFTDEP.
It might be reasonable to move the change of fs_ronly to under MNT_ILOCK,
but no readers take it.

Reported and tested by:	pho
Reviewed by:	mckusick
Sponsored by:	The FreeBSD Foundation
MFC after:	2 weeks
Differential revision:	https://reviews.freebsd.org/D29178
2021-03-12 13:31:07 +02:00
Konstantin Belousov
7f682bdcab Rework MOUNTED/DOING SOFTDEP/SUJ macros
Now MNT_SOFTDEP indicates that SU are active in any variant +-J, and
SU+J is indicated by MNT_SOFTDEP | MNT_SUJ combination.  The reason is
that unmount will be able to easily hide SU from other operations by
clearing MNT_SOFTDEP while keeping the record of the active journal.

Reviewed by:	mckusick
Tested by:	pho
Sponsored by:	The FreeBSD Foundation
MFC after:	2 weeks
Differential revision:	https://reviews.freebsd.org/D29178
2021-03-12 13:31:07 +02:00