Date: Wed, 26 Oct 2016 20:28:23 +0000 (UTC) From: Kirk McKusick <mckusick@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r307978 - head/sys/ufs/ufs Message-ID: <201610262028.u9QKSNpN003647@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: mckusick Date: Wed Oct 26 20:28:23 2016 New Revision: 307978 URL: https://svnweb.freebsd.org/changeset/base/307978 Log: The UFS/FFS filesystem checks directory link counts when doing directory create and delete operations. If it ever finds a directory with a link count less than 2, it panics. Thus, an rm -rf that encounters a directory with a link count below 2 causes a kernel panic. The proposed fix is to return the error EINVAL rather than panicing. The effect is that the requested operation is not done, but the system continues to run. At a more convenient later time, the filesystem can be unmounted and cleaned (with fsck or journal run). Once cleaned, the operation can be rerun to successful completion. This fix takes that approach. The panic message has been converted into a uprintf(9) to provide the user with the inode number and filesystem mount point of the offending directory and EINVAL is returned for the operation. The long (three year) delay in fixing this problem occurred because the bug was misclassified when originally assigned and only this week was found during a sweep of old unresolved bug reports. PR: 180894 Reviewed by: kib MFC after: 2 weeks Modified: head/sys/ufs/ufs/ufs_vnops.c Modified: head/sys/ufs/ufs/ufs_vnops.c ============================================================================== --- head/sys/ufs/ufs/ufs_vnops.c Wed Oct 26 20:12:30 2016 (r307977) +++ head/sys/ufs/ufs/ufs_vnops.c Wed Oct 26 20:28:23 2016 (r307978) @@ -105,7 +105,7 @@ static vop_create_t ufs_create; static vop_getattr_t ufs_getattr; static vop_ioctl_t ufs_ioctl; static vop_link_t ufs_link; -static int ufs_makeinode(int mode, struct vnode *, struct vnode **, struct componentname *); +static int ufs_makeinode(int mode, struct vnode *, struct vnode **, struct componentname *, const char *); static vop_markatime_t ufs_markatime; static vop_mkdir_t ufs_mkdir; static vop_mknod_t ufs_mknod; @@ -204,7 +204,7 @@ ufs_create(ap) error = ufs_makeinode(MAKEIMODE(ap->a_vap->va_type, ap->a_vap->va_mode), - ap->a_dvp, ap->a_vpp, ap->a_cnp); + ap->a_dvp, ap->a_vpp, ap->a_cnp, "ufs_create"); if (error != 0) return (error); if ((ap->a_cnp->cn_flags & MAKEENTRY) != 0) @@ -232,7 +232,7 @@ ufs_mknod(ap) int error; error = ufs_makeinode(MAKEIMODE(vap->va_type, vap->va_mode), - ap->a_dvp, vpp, ap->a_cnp); + ap->a_dvp, vpp, ap->a_cnp, "ufs_mknod"); if (error) return (error); ip = VTOI(*vpp); @@ -942,6 +942,17 @@ out: return (error); } +static void +print_bad_link_count(const char *funcname, struct vnode *dvp) +{ + struct inode *dip; + + dip = VTOI(dvp); + uprintf("%s: Bad link count %d on parent inode %d in file system %s\n", + funcname, dip->i_effnlink, dip->i_number, + dvp->v_mount->mnt_stat.f_mntonname); +} + /* * link vnode call */ @@ -964,9 +975,11 @@ ufs_link(ap) if ((cnp->cn_flags & HASBUF) == 0) panic("ufs_link: no name"); #endif - if (VTOI(tdvp)->i_effnlink < 2) - panic("ufs_link: Bad link count %d on parent", - VTOI(tdvp)->i_effnlink); + if (VTOI(tdvp)->i_effnlink < 2) { + print_bad_link_count("ufs_link", tdvp); + error = EINVAL; + goto out; + } ip = VTOI(vp); if ((nlink_t)ip->i_nlink >= LINK_MAX) { error = EMLINK; @@ -1710,10 +1723,10 @@ ufs_do_posix1e_acl_inheritance_file(stru * XXX: This should not happen, as EOPNOTSUPP above was * supposed to free acl. */ - printf("ufs_makeinode: VOP_GETACL() but no " - "VOP_SETACL()\n"); - /* panic("ufs_makeinode: VOP_GETACL() but no " - "VOP_SETACL()"); */ + printf("ufs_do_posix1e_acl_inheritance_file: VOP_GETACL() " + "but no VOP_SETACL()\n"); + /* panic("ufs_do_posix1e_acl_inheritance_file: VOP_GETACL() " + "but no VOP_SETACL()"); */ break; default: @@ -1791,6 +1804,11 @@ ufs_mkdir(ap) * but not have it entered in the parent directory. The entry is * made later after writing "." and ".." entries. */ + if (dp->i_effnlink < 2) { + print_bad_link_count("ufs_mkdir", dvp); + error = EINVAL; + goto out; + } error = UFS_VALLOC(dvp, dmode, cnp->cn_cred, &tvp); if (error) goto out; @@ -2021,13 +2039,12 @@ ufs_rmdir(ap) * tries to remove a locally mounted on directory). */ error = 0; - if (ip->i_effnlink < 2) { + if (dp->i_effnlink <= 2) { + if (dp->i_effnlink == 2) + print_bad_link_count("ufs_rmdir", dvp); error = EINVAL; goto out; } - if (dp->i_effnlink < 3) - panic("ufs_dirrem: Bad link count %d on parent", - dp->i_effnlink); if (!ufs_dirempty(ip, dp->i_number, cnp->cn_cred)) { error = ENOTEMPTY; goto out; @@ -2106,7 +2123,7 @@ ufs_symlink(ap) int len, error; error = ufs_makeinode(IFLNK | ap->a_vap->va_mode, ap->a_dvp, - vpp, ap->a_cnp); + vpp, ap->a_cnp, "ufs_symlink"); if (error) return (error); vp = *vpp; @@ -2558,11 +2575,12 @@ ufs_vinit(mntp, fifoops, vpp) * Vnode dvp must be locked. */ static int -ufs_makeinode(mode, dvp, vpp, cnp) +ufs_makeinode(mode, dvp, vpp, cnp, callfunc) int mode; struct vnode *dvp; struct vnode **vpp; struct componentname *cnp; + const char *callfunc; { struct inode *ip, *pdir; struct direct newdir; @@ -2572,15 +2590,16 @@ ufs_makeinode(mode, dvp, vpp, cnp) pdir = VTOI(dvp); #ifdef INVARIANTS if ((cnp->cn_flags & HASBUF) == 0) - panic("ufs_makeinode: no name"); + panic("%s: no name", callfunc); #endif *vpp = NULL; if ((mode & IFMT) == 0) mode |= IFREG; - if (VTOI(dvp)->i_effnlink < 2) - panic("ufs_makeinode: Bad link count %d on parent", - VTOI(dvp)->i_effnlink); + if (pdir->i_effnlink < 2) { + print_bad_link_count(callfunc, dvp); + return (EINVAL); + } error = UFS_VALLOC(dvp, mode, cnp->cn_cred, &tvp); if (error) return (error);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201610262028.u9QKSNpN003647>