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

Avoid LOR between vfs_busy() lock and covered vnode lock on quotaon().

Browse Source 
The vfs_busy() is after covered vnode lock in the global lock order, but
since quotaon() does recursive VFS call to open quota file, we usually
end up locking covered vnode after mp is busied in sys_quotactl().

Change the interface of VFS_QUOTACTL(), requiring that mp was unbusied
by fs code, and do not try to pick up vfs_busy() reference in ufs quotaon,
esp. if vfs_busy cannot succeed due to unmount being performed.

Reported and tested by:	pho
MFC after:	1 week
This commit is contained in:
Konstantin Belousov 2012-01-08 23:06:53 +00:00
parent a194b02d88
commit 3ab0160340
2 changed files with 34 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
sys/kern/vfs_syscalls.c
View File

@ -86,6 +86,8 @@ __FBSDID("$FreeBSD$");
#include <vm/vm_page.h>
#include <vm/uma.h>
#include <ufs/ufs/quota.h>
static MALLOC_DEFINE(M_FADVISE, "fadvise", "posix_fadvise(2) information");
SDT_PROVIDER_DEFINE(vfs);
@ -212,7 +214,20 @@ sys_quotactl(td, uap)
return (error);
}
error = VFS_QUOTACTL(mp, uap->cmd, uap->uid, uap->arg);
vfs_unbusy(mp);
/*
* Since quota on operation typically needs to open quota
* file, the Q_QUOTAON handler needs to unbusy the mount point
* before calling into namei. Otherwise, unmount might be
* started between two vfs_busy() invocations (first is our,
* second is from mount point cross-walk code in lookup()),
* causing deadlock.
*
* Require that Q_QUOTAON handles the vfs_busy() reference on
* its own, always returning with ubusied mount point.
*/
if ((uap->cmd >> SUBCMDSHIFT) != Q_QUOTAON)
vfs_unbusy(mp);
VFS_UNLOCK_GIANT(vfslocked);
return (error);
}

21
sys/ufs/ufs/ufs_quota.c
View File

@ -512,17 +512,29 @@ quotaon(struct thread *td, struct mount *mp, int type, void *fname)
NDINIT(&nd, LOOKUP, FOLLOW | MPSAFE, UIO_USERSPACE, fname, td);
flags = FREAD | FWRITE;
vfs_ref(mp);
vfs_unbusy(mp);
error = vn_open(&nd, &flags, 0, NULL);
if (error)
if (error != 0) {
vfs_rel(mp);
return (error);
}
vfslocked = NDHASGIANT(&nd);
NDFREE(&nd, NDF_ONLY_PNBUF);
vp = nd.ni_vp;
if (vp->v_type != VREG) {
error = vfs_busy(mp, MBF_NOWAIT);
vfs_rel(mp);
if (error == 0) {
if (vp->v_type != VREG) {
error = EACCES;
vfs_unbusy(mp);
}
}
if (error != 0) {
VOP_UNLOCK(vp, 0);
(void) vn_close(vp, FREAD|FWRITE, td->td_ucred, td);
VFS_UNLOCK_GIANT(vfslocked);
return (EACCES);
return (error);
}
UFS_LOCK(ump);
@ -531,6 +543,7 @@ quotaon(struct thread *td, struct mount *mp, int type, void *fname)
VOP_UNLOCK(vp, 0);
(void) vn_close(vp, FREAD|FWRITE, td->td_ucred, td);
VFS_UNLOCK_GIANT(vfslocked);
vfs_unbusy(mp);
return (EALREADY);
}
ump->um_qflags[type] |= QTF_OPENING|QTF_CLOSING;
@ -542,6 +555,7 @@ quotaon(struct thread *td, struct mount *mp, int type, void *fname)
UFS_UNLOCK(ump);
(void) vn_close(vp, FREAD|FWRITE, td->td_ucred, td);
VFS_UNLOCK_GIANT(vfslocked);
vfs_unbusy(mp);
return (error);
}
VOP_UNLOCK(vp, 0);
@ -619,6 +633,7 @@ quotaon(struct thread *td, struct mount *mp, int type, void *fname)
("quotaon: leaking flags"));
UFS_UNLOCK(ump);
vfs_unbusy(mp);
return (error);
}