Project

General

Profile

Bug #3983

sdev_shadow_node slept through a vn_rele

Added by Robert Mustacchi over 6 years ago. Updated over 6 years ago.

Status:
Resolved
Priority:
Normal
Category:
kernel
Start date:
2013-08-04
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:

Description

Fundamentally the code in sdev is buggy. The best way to look at this is to focus on the sdev_shadow_node function. It is reproduced in part below:

1608 lookup:
1609         /* try to find it in the backing store */
1610         error = VOP_LOOKUP(rdvp, nm, rvp, NULL, 0, NULL, cred, NULL, NULL,
1611             NULL);
1612         if (error == 0) {
1613                 if (VOP_REALVP(*rvp, &rrvp, NULL) == 0) {
1614                         VN_HOLD(rrvp);
1615                         VN_RELE(*rvp);
1616                         *rvp = rrvp;
1617                 }
1618 
1619                 kmem_free(dv->sdev_attr, sizeof (vattr_t));
1620                 dv->sdev_attr = NULL;
1621                 dv->sdev_attrvp = *rvp;
1622                 return (0);
1623         }
1624 
1625         /* let's try to persist the node */
1626         gethrestime(&vap->va_atime);
1627         vap->va_mtime = vap->va_atime;
1628         vap->va_ctime = vap->va_atime;
1629         vap->va_mask |= AT_TYPE|AT_MODE;
1630         switch (vap->va_type) {
1631         case VDIR:
1632                 error = VOP_MKDIR(rdvp, nm, vap, rvp, cred, NULL, 0, NULL);
1633                 sdcmn_err9(("sdev_shadow_node: mkdir vp %p error %d\\n",
1634                     (void *)(*rvp), error));
1635                 break;
1636         case VCHR:
1637         case VBLK:
1638         case VREG:
1639         case VDOOR:
1640                 error = VOP_CREATE(rdvp, nm, vap, NONEXCL, VREAD|VWRITE,
1641                     rvp, cred, 0, NULL, NULL);
1642                 sdcmn_err9(("sdev_shadow_node: create vp %p, error %d\\n",
1643                     (void *)(*rvp), error));
1644                 if (!error)
1645                         VN_RELE(*rvp);
1646                 break;
1647         case VLNK:
1648                 ASSERT(dv->sdev_symlink);
1649                 error = VOP_SYMLINK(rdvp, nm, vap, dv->sdev_symlink, cred,
1650                     NULL, 0);
1651                 sdcmn_err9(("sdev_shadow_node: create symlink error %d\\n",
1652                     error));
1653                 break;
1654         default:
1655                 cmn_err(CE_PANIC, "dev: %s: sdev_shadow_node " 
1656                     "create\\n", nm);
1657                 /*NOTREACHED*/
1658         }
1659 
1660         /* go back to lookup to factor out spec node and set attrvp */
1661         if (error == 0)
1662                 goto lookup;

If we already have the node in question then we do the lookup and everything is fine. But here's where it gets bad. When we go and create the node we take a reference on the vnode. Then we go back later and do a lookup on the vnode which itself adds another reference. In most cases we end up doing an unref on the created node which is appropriate because we're not using it. However in the case of a directory or a symlink we don't end up dropping the reference and thus end up leaking the node.

History

#1

Updated by Robert Mustacchi over 6 years ago

  • Status changed from New to Resolved

Resolved in 9e5aa9d8a21f8efa8ba9c9e4a0aa6edc66d07eb2.

#2

Updated by Robert Mustacchi over 6 years ago

Resolved in 9e5aa9d8a21f8efa8ba9c9e4a0aa6edc66d07eb2.

Also available in: Atom PDF