vfs_lookup_cross_mount(): Fix reference counting/locking on LK_UPGRADE error
ClosedPublic
Actions

Authored by olce on Sep 5 2023, 12:58 PM.

Details

Reviewers

mjg
kib
jah

Commits

rGaaeaba324335: vfs: fix reference counting/locking on LK_UPGRADE error
rGc76dfb929e70: vfs: fix reference counting/locking on LK_UPGRADE error
rG02cbc029dac9: vfs: fix reference counting/locking on LK_UPGRADE error

Summary

Factoring out this code unfortunately introduced reference and lock leaks in
case of failure in the lock upgrade path under VV_CROSSLOCK. In terms of
practical use, this impacts unionfs (and nullfs in a corner case).

Also, add under INVARIANTS a check about the return value of vn_lock() (it is
not supposed to be anything else than ENOENT).

Fixes: 80bd5ef07025 vfs: factor out mount point traversal to a dedicated routine
MFC after: 3 days
MFC to: stable/14 releng/14.0
Sponsored by: The FreeBSD Foundation

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

olce created this revision.Sep 5 2023, 12:58 PM

Herald added a subscriber: imp. · View Herald TranscriptSep 5 2023, 12:58 PM

olce requested review of this revision.Sep 5 2023, 12:58 PM

Harbormaster completed remote builds in B53435: Diff 126882.Sep 5 2023, 12:58 PM

Add an MPASS() on error, change a bit the commit message.

Harbormaster completed remote builds in B53477: Diff 126986.Sep 6 2023, 7:02 PM

olce added reviewers: mjg, kib, jah.Sep 6 2023, 7:03 PM

olce mentioned this in D40883: vfs: factor out mount point traversal to a dedicated routine.

olce added subscribers: emaste, markj.

olce edited the summary of this revision. (Show Details)Sep 6 2023, 7:17 PM

I found the bug around hte next day after introducing it, it must have slipped through the cracks afterwards.

I'll sort this review out over the weekend, thanks for submitting.

This revision was not accepted when it landed; it landed in state Needs Review.Sep 22 2023, 9:00 PM

Closed by commit rG02cbc029dac9: vfs: fix reference counting/locking on LK_UPGRADE error (authored by olce, committed by mjg). · Explain Why

This revision was automatically updated to reflect the committed changes.

mjg added a commit: rG02cbc029dac9: vfs: fix reference counting/locking on LK_UPGRADE error.

mjg added a commit: rGc76dfb929e70: vfs: fix reference counting/locking on LK_UPGRADE error.Sep 24 2023, 10:45 AM

mjg added a commit: rGaaeaba324335: vfs: fix reference counting/locking on LK_UPGRADE error.Sep 24 2023, 9:47 PM

Revision Contents
Changeset List

Path

Size

sys/

kern/

vfs_lookup.c

9 lines

Diff 127655

View Options

sys/kern/vfs_lookup.c

	# Created by: Wen Heping <wenheping@gmail.com>			# Created by: Wen Heping <wenheping@gmail.com>
	# $FreeBSD$			# $FreeBSD$

	PORTNAME= pyliblo			PORTNAME= pyliblo
	PORTVERSION= 0.10.0			DISTVERSION= 0.10.0
				PORTREVISION= 1
	CATEGORIES= audio python			CATEGORIES= audio python
	MASTER_SITES= http://das.nasophon.de/download/			MASTER_SITES= http://das.nasophon.de/download/
	PKGNAMEPREFIX= ${PYTHON_PKGNAMEPREFIX}			PKGNAMEPREFIX= ${PYTHON_PKGNAMEPREFIX}

	MAINTAINER= ports@FreeBSD.org			MAINTAINER= ports@FreeBSD.org
	COMMENT= Python wrapper for the Liblo OSC Library			COMMENT= Python wrapper for the Liblo OSC Library

	LICENSE= LGPL21			LICENSE= LGPL21
	LICENSE_FILE= ${WRKSRC}/COPYING			LICENSE_FILE= ${WRKSRC}/COPYING

	BUILD_DEPENDS= pyrex>=0.9.4:devel/pyrex \			BUILD_DEPENDS= cython${PYTHON_VER:M3.*:S/^3/-3/}:lang/cython${PYTHON_MAJOR_VER:M3}
	${LOCALBASE}/bin/cython:lang/cython
	LIB_DEPENDS= liblo.so:audio/liblo			LIB_DEPENDS= liblo.so:audio/liblo

				USES= localbase:ldflags python
	USES= python			USES= python
	USE_PYTHON= distutils			USE_PYTHON= distutils # autoplist fails to add the manpage files
				matUnsubmitted Not Done Inline Actions I don't understand the comment here. `USE_PYTHON=autoplist` looks at what files `distutils` has installed and uses that to generate the plist. In this port, the man pages are installed manually in post-install, so there is no way that distutils knows about them, so it seems normal that they are not present in the plist generated by autoplist. mat: I don't understand the comment here. `USE_PYTHON=autoplist` looks at what files `distutils`…
	MAKE_ENV+= PREFIX=${PREFIX} LOCALBASE=${LOCALBASE}			MAKE_ENV+= PREFIX=${PREFIX} LOCALBASE=${LOCALBASE}
	LDFLAGS+= -L${LOCALBASE}/lib

	post-patch:			post-patch:
	${REINPLACE_CMD} -e 's#%%LOCALBASE%%#${LOCALBASE}#g' ${WRKSRC}/setup.py			${REINPLACE_CMD} -e 's#%%LOCALBASE%%#${LOCALBASE}#g' ${WRKSRC}/setup.py

	post-install:			post-install:
	${STRIP_CMD} ${STAGEDIR}${PYTHONPREFIX_SITELIBDIR}/liblo.so			${STRIP_CMD} ${STAGEDIR}${PYTHONPREFIX_SITELIBDIR}/liblo.so
	${INSTALL_MAN} ${WRKSRC}/scripts/dump_osc.1 ${STAGEDIR}${MAN1PREFIX}/man/man1			${INSTALL_MAN} ${WRKSRC}/scripts/dump_osc.1 ${STAGEDIR}${MAN1PREFIX}/man/man1
	${INSTALL_MAN} ${WRKSRC}/scripts/send_osc.1 ${STAGEDIR}${MAN1PREFIX}/man/man1			${INSTALL_MAN} ${WRKSRC}/scripts/send_osc.1 ${STAGEDIR}${MAN1PREFIX}/man/man1

	.include <bsd.port.mk>			.include <bsd.port.mk>

vfs_lookup_cross_mount(): Fix reference counting/locking on LK_UPGRADE errorClosedPublicActions