Do not upgrade the vnode lock to call getinoquota().

Doing so can deadlock when the thread already owns another vnode lock,
e.g. during a rename, as was demonstrated by the reporter.  In fact,
there seems to be no need to force the call to getinoquota() always,
because vn_open() locks vnode exclusively, and this is the most
important case.  To add to the point, directories where the dirent is
added or removed, are locked exclusively as well.

Reported by:	bwidawsk
Tested by:	bwidawsk, pho (as part of the larger patch)
Sponsored by:	The FreeBSD Foundation
Approved by:	re (gjb)
MFC after:	1 week
This commit is contained in:
Konstantin Belousov 2018-09-17 19:38:43 +00:00
parent 77e4a39103
commit a408841593

View File

@ -325,9 +325,6 @@ ufs_accessx(ap)
struct inode *ip = VTOI(vp);
accmode_t accmode = ap->a_accmode;
int error;
#ifdef QUOTA
int relocked;
#endif
#ifdef UFS_ACL
struct acl *acl;
acl_type_t type;
@ -350,32 +347,14 @@ ufs_accessx(ap)
* Inode is accounted in the quotas only if struct
* dquot is attached to it. VOP_ACCESS() is called
* from vn_open_cred() and provides a convenient
* point to call getinoquota().
* point to call getinoquota(). The lock mode is
* exclusive when the file is opening for write.
*/
if (VOP_ISLOCKED(vp) != LK_EXCLUSIVE) {
/*
* Upgrade vnode lock, since getinoquota()
* requires exclusive lock to modify inode.
*/
relocked = 1;
vhold(vp);
vn_lock(vp, LK_UPGRADE | LK_RETRY);
VI_LOCK(vp);
if (vp->v_iflag & VI_DOOMED) {
vdropl(vp);
error = ENOENT;
goto relock;
}
vdropl(vp);
} else
relocked = 0;
error = getinoquota(ip);
relock:
if (relocked)
vn_lock(vp, LK_DOWNGRADE | LK_RETRY);
if (error != 0)
return (error);
if (VOP_ISLOCKED(vp) == LK_EXCLUSIVE) {
error = getinoquota(ip);
if (error != 0)
return (error);
}
#endif
break;
default: