D33729.id101057.diff
No OneTemporary
Actions

Size

15 KB

Referenced Files

None

Subscribers

None

D33729.id101057.diff
View Options

	Index: sys/fs/unionfs/union_subr.c
	===================================================================
	--- sys/fs/unionfs/union_subr.c
	+++ sys/fs/unionfs/union_subr.c
	@@ -525,8 +525,9 @@
	}

	/*
	- * Get the unionfs node status.
	- * You need exclusive lock this vnode.
	+ * Get the unionfs node status object for the vnode corresponding to unp,
	+ * for the process that owns td. Allocate a new status object if one
	+ * does not already exist.
	*/
	void
	unionfs_get_node_status(struct unionfs_node unp, struct thread td,
	Index: sys/fs/unionfs/union_vnops.c
	===================================================================
	--- sys/fs/unionfs/union_vnops.c
	+++ sys/fs/unionfs/union_vnops.c
	@@ -470,30 +470,73 @@
	return (error);
	}

	+enum unionfs_lkupgrade {
	+ UNIONFS_LKUPGRADE_SUCCESS, /* lock successfully upgraded */
	+ UNIONFS_LKUPGRADE_ALREADY, /* lock already held exclusive */
	+ UNIONFS_LKUPGRADE_DOOMED /* lock was upgraded, but vnode reclaimed */
	+};
	+
	+static inline enum unionfs_lkupgrade
	+unionfs_upgrade_lock(struct vnode *vp)
	+{
	+ ASSERT_VOP_LOCKED(vp, __func__);
	+
	+ if (VOP_ISLOCKED(vp) == LK_EXCLUSIVE)
	+ return (UNIONFS_LKUPGRADE_ALREADY);
	+
	+ if (vn_lock(vp, LK_UPGRADE) != 0) {
	+ vn_lock(vp, LK_EXCLUSIVE \| LK_RETRY);
	+ if (VN_IS_DOOMED(vp))
	+ return (UNIONFS_LKUPGRADE_DOOMED);
	+ }
	+ return (UNIONFS_LKUPGRADE_SUCCESS);
	+}
	+
	+static inline void
	+unionfs_downgrade_lock(struct vnode *vp, enum unionfs_lkupgrade status)
	+{
	+ if (status != UNIONFS_LKUPGRADE_ALREADY)
	+ vn_lock(vp, LK_DOWNGRADE \| LK_RETRY);
	+}
	+
	static int
	unionfs_open(struct vop_open_args *ap)
	{
	struct unionfs_node *unp;
	struct unionfs_node_status *unsp;
	+ struct vnode *vp;
	struct vnode *uvp;
	struct vnode *lvp;
	struct vnode *targetvp;
	struct ucred *cred;
	struct thread *td;
	int error;
	+ enum unionfs_lkupgrade lkstatus;

	UNIONFS_INTERNAL_DEBUG("unionfs_open: enter\n");

	KASSERT_UNIONFS_VNODE(ap->a_vp);

	error = 0;
	- unp = VTOUNIONFS(ap->a_vp);
	- uvp = unp->un_uppervp;
	- lvp = unp->un_lowervp;
	+ vp = ap->a_vp;
	targetvp = NULLVP;
	cred = ap->a_cred;
	td = ap->a_td;

	+ /*
	+ * The executable loader path may call this function with vp locked
	+ * shared. If the vnode is reclaimed while upgrading, we can't safely
	+ * use unp or do anything else unionfs- specific.
	+ */
	+ lkstatus = unionfs_upgrade_lock(vp);
	+ if (lkstatus == UNIONFS_LKUPGRADE_DOOMED) {
	+ error = ENOENT;
	+ goto unionfs_open_cleanup;
	+ }
	+
	+ unp = VTOUNIONFS(vp);
	+ uvp = unp->un_uppervp;
	+ lvp = unp->un_lowervp;
	unionfs_get_node_status(unp, td, &unsp);

	if (unsp->uns_lower_opencnt > 0 \|\| unsp->uns_upper_opencnt > 0) {
	@@ -540,13 +583,16 @@
	unsp->uns_lower_opencnt++;
	unsp->uns_lower_openmode = ap->a_mode;
	}
	- ap->a_vp->v_object = targetvp->v_object;
	+ vp->v_object = targetvp->v_object;
	}

	unionfs_open_abort:
	if (error != 0)
	unionfs_tryrem_node_status(unp, unsp);

	+unionfs_open_cleanup:
	+ unionfs_downgrade_lock(vp, lkstatus);
	+
	UNIONFS_INTERNAL_DEBUG("unionfs_open: leave (%d)\n", error);

	return (error);
	@@ -562,23 +608,26 @@
	struct vnode *vp;
	struct vnode *ovp;
	int error;
	- int locked;
	+ enum unionfs_lkupgrade lkstatus;;

	UNIONFS_INTERNAL_DEBUG("unionfs_close: enter\n");

	KASSERT_UNIONFS_VNODE(ap->a_vp);

	- locked = 0;
	vp = ap->a_vp;
	- unp = VTOUNIONFS(vp);
	cred = ap->a_cred;
	td = ap->a_td;
	+ error = 0;

	- if (VOP_ISLOCKED(vp) != LK_EXCLUSIVE) {
	- if (vn_lock(vp, LK_UPGRADE) != 0)
	- vn_lock(vp, LK_EXCLUSIVE \| LK_RETRY);
	- locked = 1;
	- }
	+ /*
	+ * If the vnode is reclaimed while upgrading, we can't safely use unp
	+ * or do anything else unionfs- specific.
	+ */
	+ lkstatus = unionfs_upgrade_lock(vp);
	+ if (lkstatus == UNIONFS_LKUPGRADE_DOOMED)
	+ goto unionfs_close_cleanup;
	+
	+ unp = VTOUNIONFS(vp);
	unionfs_get_node_status(unp, td, &unsp);

	if (unsp->uns_lower_opencnt <= 0 && unsp->uns_upper_opencnt <= 0) {
	@@ -618,8 +667,8 @@
	unionfs_close_abort:
	unionfs_tryrem_node_status(unp, unsp);

	- if (locked != 0)
	- vn_lock(vp, LK_DOWNGRADE \| LK_RETRY);
	+unionfs_close_cleanup:
	+ unionfs_downgrade_lock(vp, lkstatus);

	UNIONFS_INTERNAL_DEBUG("unionfs_close: leave (%d)\n", error);

	@@ -1444,6 +1493,11 @@
	VNPASS(vrefcnt(ap->a_vp) > 0, ap->a_vp);
	error = unionfs_relookup_for_delete(ap->a_dvp, cnp, td);
	vn_lock(ap->a_vp, LK_EXCLUSIVE \| LK_RETRY);
	+ /*
	+ * VOP_RMDIR is dispatched against udvp, so if uvp became
	+ * doomed while the lock was dropped above the target
	+ * filesystem may not be able to cope.
	+ */
	if (error == 0 && VN_IS_DOOMED(uvp))
	error = ENOENT;
	if (error == 0)
	@@ -1514,9 +1568,9 @@
	uint64_t *cookies_bk;
	int error;
	int eofflag;
	- int locked;
	int ncookies_bk;
	int uio_offset_bk;
	+ enum unionfs_lkupgrade lkstatus;

	UNIONFS_INTERNAL_DEBUG("unionfs_readdir: enter\n");

	@@ -1524,7 +1578,6 @@

	error = 0;
	eofflag = 0;
	- locked = 0;
	uio_offset_bk = 0;
	uio = ap->a_uio;
	uvp = NULLVP;
	@@ -1537,18 +1590,18 @@
	if (vp->v_type != VDIR)
	return (ENOTDIR);

	- /* check the open count. unionfs needs to open before readdir. */
	- if (VOP_ISLOCKED(vp) != LK_EXCLUSIVE) {
	- if (vn_lock(vp, LK_UPGRADE) != 0)
	- vn_lock(vp, LK_EXCLUSIVE \| LK_RETRY);
	- locked = 1;
	- }
	- unp = VTOUNIONFS(vp);
	- if (unp == NULL)
	+ /*
	+ * If the vnode is reclaimed while upgrading, we can't safely use unp
	+ * or do anything else unionfs- specific.
	+ */
	+ lkstatus = unionfs_upgrade_lock(vp);
	+ if (lkstatus == UNIONFS_LKUPGRADE_DOOMED)
	error = EBADF;
	- else {
	+ if (error == 0) {
	+ unp = VTOUNIONFS(vp);
	uvp = unp->un_uppervp;
	lvp = unp->un_lowervp;
	+ /* check the open count. unionfs needs open before readdir. */
	unionfs_get_node_status(unp, td, &unsp);
	if ((uvp != NULLVP && unsp->uns_upper_opencnt <= 0) \|\|
	(lvp != NULLVP && unsp->uns_lower_opencnt <= 0)) {
	@@ -1556,8 +1609,7 @@
	error = EBADF;
	}
	}
	- if (locked)
	- vn_lock(vp, LK_DOWNGRADE \| LK_RETRY);
	+ unionfs_downgrade_lock(vp, lkstatus);
	if (error != 0)
	goto unionfs_readdir_exit;

	@@ -1906,7 +1958,8 @@
	if ((revlock = unionfs_get_llt_revlock(vp, flags)) == 0)
	panic("unknown lock type: 0x%x", flags & LK_TYPE_MASK);

	- if ((vp->v_iflag & VI_OWEINACT) != 0)
	+ if ((flags & LK_TYPE_MASK) != LK_DOWNGRADE &&
	+ (vp->v_iflag & VI_OWEINACT) != 0)
	flags \|= LK_NOWAIT;

	/*
	@@ -2251,10 +2304,12 @@
	if (error == 0) {
	if (vn_lock(vp, LK_UPGRADE) != 0)
	vn_lock(vp, LK_EXCLUSIVE \| LK_RETRY);
	- if (tvp == unp->un_uppervp)
	- unp->un_flag \|= UNIONFS_OPENEXTU;
	- else
	- unp->un_flag \|= UNIONFS_OPENEXTL;
	+ if (!VN_IS_DOOMED(vp)) {
	+ if (tvp == unp->un_uppervp)
	+ unp->un_flag \|= UNIONFS_OPENEXTU;
	+ else
	+ unp->un_flag \|= UNIONFS_OPENEXTL;
	+ }
	vn_lock(vp, LK_DOWNGRADE \| LK_RETRY);
	}

	@@ -2288,10 +2343,12 @@
	if (error == 0) {
	if (vn_lock(vp, LK_UPGRADE) != 0)
	vn_lock(vp, LK_EXCLUSIVE \| LK_RETRY);
	- if (tvp == unp->un_uppervp)
	- unp->un_flag &= ~UNIONFS_OPENEXTU;
	- else
	- unp->un_flag &= ~UNIONFS_OPENEXTL;
	+ if (!VN_IS_DOOMED(vp)) {
	+ if (tvp == unp->un_uppervp)
	+ unp->un_flag &= ~UNIONFS_OPENEXTU;
	+ else
	+ unp->un_flag &= ~UNIONFS_OPENEXTL;
	+ }
	vn_lock(vp, LK_DOWNGRADE \| LK_RETRY);
	}

	Index: tools/test/stress2/misc/unionfs10.sh
	===================================================================
	--- /dev/null
	+++ tools/test/stress2/misc/unionfs10.sh
	@@ -0,0 +1,87 @@
	+#!/bin/sh
	+
	+#
	+# Copyright (c) 2022 Jason Harmening <jah@FreeBSD.org>
	+# All rights reserved.
	+#
	+# Redistribution and use in source and binary forms, with or without
	+# modification, are permitted provided that the following conditions
	+# are met:
	+# 1. Redistributions of source code must retain the above copyright
	+# notice, this list of conditions and the following disclaimer.
	+# 2. Redistributions in binary form must reproduce the above copyright
	+# notice, this list of conditions and the following disclaimer in the
	+# documentation and/or other materials provided with the distribution.
	+#
	+# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
	+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
	+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
	+# ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
	+# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
	+# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
	+# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
	+# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
	+# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
	+# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
	+# SUCH DAMAGE.
	+#
	+
	+[ `id -u ` -ne 0 ] && echo "Must be root!" && exit 1
	+
	+# Regression test:
	+
	+# There is an issue, namely, when the lower file is already
	+# opened r/w, but its nullfs alias is executed. This situation obviously
	+# shall result in ETXTBUSY, but it currently does not.
	+
	+# Based on nullfs10.sh by pho@, original test scenario by kib@
	+
	+. ../default.cfg
	+
	+md1=$mdstart
	+md2=$((md1 + 1))
	+mp1=/mnt$md1
	+mp2=/mnt$md2
	+mkdir -p $mp1 $mp2
	+set -e
	+for i in $mp1 $mp2; do
	+ mount \| grep -q "on $i " && umount -f $i
	+done
	+for i in $md1 $md2; do
	+ mdconfig -l \| grep -q md$i && mdconfig -d -u $i
	+done
	+
	+mdconfig -a -t swap -s 4g -u $md1
	+mdconfig -a -t swap -s 4g -u $md2
	+newfs $newfs_flags -n md$md1 > /dev/null
	+newfs $newfs_flags -n md$md2 > /dev/null
	+mount /dev/md$md1 $mp1
	+mount /dev/md$md2 $mp2
	+
	+mount -t unionfs -o noatime $mp1 $mp2
	+set +e
	+
	+mount \| grep -E "$mp1\|$mp2"
	+
	+chmod 777 $mp1
	+chmod 777 $mp2
	+
	+cp /bin/ls $mp1
	+chmod +w $mp1/ls
	+sleep 2 >> $mp1/ls &
	+sleep .5
	+
	+# This line should cause a "$mp2/ls: Text file busy" error
	+$mp2/ls -l /bin/ls $mp1 $mp2 && echo FAIL \|\| echo OK
	+kill $!
	+wait
	+
	+while mount \| grep -q "$mp2 "; do
	+ umount $mp2 \|\| sleep 1
	+done
	+
	+while mount \| grep -q "$mp1 "; do
	+ umount $mp1 \|\| sleep 1
	+done
	+mdconfig -d -u $md2
	+mdconfig -d -u $md1
	Index: tools/test/stress2/misc/unionfs11.sh
	===================================================================
	--- /dev/null
	+++ tools/test/stress2/misc/unionfs11.sh
	@@ -0,0 +1,87 @@
	+#!/bin/sh
	+
	+#
	+# Copyright (c) 2022 Jason Harmening <jah@FreeBSD.org>
	+# All rights reserved.
	+#
	+# Redistribution and use in source and binary forms, with or without
	+# modification, are permitted provided that the following conditions
	+# are met:
	+# 1. Redistributions of source code must retain the above copyright
	+# notice, this list of conditions and the following disclaimer.
	+# 2. Redistributions in binary form must reproduce the above copyright
	+# notice, this list of conditions and the following disclaimer in the
	+# documentation and/or other materials provided with the distribution.
	+#
	+# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
	+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
	+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
	+# ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
	+# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
	+# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
	+# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
	+# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
	+# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
	+# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
	+# SUCH DAMAGE.
	+#
	+
	+[ `id -u ` -ne 0 ] && echo "Must be root!" && exit 1
	+
	+# Regression test:
	+
	+# There is an issue, namely, when the lower file is already
	+# opened r/w, but its nullfs alias is executed. This situation obviously
	+# shall result in ETXTBUSY, but it currently does not.
	+
	+# Based on nullfs10.sh by pho@, original test scenario by kib@
	+
	+. ../default.cfg
	+
	+md1=$mdstart
	+md2=$((md1 + 1))
	+mp1=/mnt$md1
	+mp2=/mnt$md2
	+mkdir -p $mp1 $mp2
	+set -e
	+for i in $mp1 $mp2; do
	+ mount \| grep -q "on $i " && umount -f $i
	+done
	+for i in $md1 $md2; do
	+ mdconfig -l \| grep -q md$i && mdconfig -d -u $i
	+done
	+
	+mdconfig -a -t swap -s 4g -u $md1
	+mdconfig -a -t swap -s 4g -u $md2
	+newfs $newfs_flags -n md$md1 > /dev/null
	+newfs $newfs_flags -n md$md2 > /dev/null
	+mount /dev/md$md1 $mp1
	+mount /dev/md$md2 $mp2
	+
	+mount -t unionfs -o noatime $mp1 $mp2
	+set +e
	+
	+mount \| grep -E "$mp1\|$mp2"
	+
	+chmod 777 $mp1
	+chmod 777 $mp2
	+
	+cp /bin/ls $mp2
	+chmod +w $mp2/ls
	+sleep 2 >> $mp2/ls &
	+sleep .5
	+
	+# This line should cause a "$mp1/ls: Text file busy" error
	+$mp1/ls -l /bin/ls $mp1 $mp2 && echo FAIL \|\| echo OK
	+kill $!
	+wait
	+
	+while mount \| grep -q "$mp2 "; do
	+ umount $mp2 \|\| sleep 1
	+done
	+
	+while mount \| grep -q "$mp1 "; do
	+ umount $mp1 \|\| sleep 1
	+done
	+mdconfig -d -u $md2
	+mdconfig -d -u $md1
	Index: tools/test/stress2/misc/unionfs12.sh
	===================================================================
	--- /dev/null
	+++ tools/test/stress2/misc/unionfs12.sh
	@@ -0,0 +1,88 @@
	+#!/bin/sh
	+
	+#
	+# Copyright (c) 2022 Jason Harmening <jah@FreeBSD.org>
	+# All rights reserved.
	+#
	+# Redistribution and use in source and binary forms, with or without
	+# modification, are permitted provided that the following conditions
	+# are met:
	+# 1. Redistributions of source code must retain the above copyright
	+# notice, this list of conditions and the following disclaimer.
	+# 2. Redistributions in binary form must reproduce the above copyright
	+# notice, this list of conditions and the following disclaimer in the
	+# documentation and/or other materials provided with the distribution.
	+#
	+# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
	+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
	+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
	+# ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
	+# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
	+# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
	+# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
	+# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
	+# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
	+# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
	+# SUCH DAMAGE.
	+#
	+
	+[ `id -u ` -ne 0 ] && echo "Must be root!" && exit 1
	+
	+# Regression test:
	+
	+# There is an issue, namely, when the lower file is already
	+# opened r/w, but its nullfs alias is executed. This situation obviously
	+# shall result in ETXTBUSY, but it currently does not.
	+
	+# Based on nullfs10.sh by pho@, original test scenario by kib@
	+
	+. ../default.cfg
	+
	+md1=$mdstart
	+md2=$((md1 + 1))
	+mp1=/mnt$md1
	+mp2=/mnt$md2
	+mkdir -p $mp1 $mp2
	+set -e
	+for i in $mp1 $mp2; do
	+ mount \| grep -q "on $i " && umount -f $i
	+done
	+for i in $md1 $md2; do
	+ mdconfig -l \| grep -q md$i && mdconfig -d -u $i
	+done
	+
	+mdconfig -a -t swap -s 4g -u $md1
	+mdconfig -a -t swap -s 4g -u $md2
	+newfs $newfs_flags -n md$md1 > /dev/null
	+newfs $newfs_flags -n md$md2 > /dev/null
	+mount /dev/md$md1 $mp1
	+mount /dev/md$md2 $mp2
	+
	+mkdir $mp2/shadow
	+cp /bin/ls $mp2/shadow/
	+mount -t unionfs -o noatime $mp1 $mp2
	+set +e
	+
	+mount \| grep -E "$mp1\|$mp2"
	+
	+chmod 777 $mp1
	+chmod 777 $mp2
	+
	+chmod +w $mp2/shadow/ls
	+sleep 2 >> $mp2/shadow/ls &
	+sleep .5
	+
	+# This line should cause a "$mp1/shadow/ls: Text file busy" error
	+$mp1/shadow/ls -l /bin/ls $mp1 $mp2 && echo FAIL \|\| echo OK
	+kill $!
	+wait
	+
	+while mount \| grep -q "$mp2 "; do
	+ umount $mp2 \|\| sleep 1
	+done
	+
	+while mount \| grep -q "$mp1 "; do
	+ umount $mp1 \|\| sleep 1
	+done
	+mdconfig -d -u $md2
	+mdconfig -d -u $md1

File Metadata

Mime Type: text/plain
Expires: Sun, Dec 21, 11:44 AM (5 h, 41 m)
Storage Engine: blob
Storage Format: Raw Data
Storage Handle: 27112495
Default Alt Text: D33729.id101057.diff (15 KB)

D33729.id101057.diffNo OneTemporaryActions

D33729.id101057.diffView Options

File Metadata

Event Timeline

D33729.id101057.diff
No OneTemporary
Actions

D33729.id101057.diff
View Options