Bug 180894 - [panic] rm -rf causes kernel panic
Summary: [panic] rm -rf causes kernel panic
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: Normal Affects Many People
Assignee: Kirk McKusick
URL:
Keywords: crash
Depends on:
Blocks:
 
Reported: 2013-07-27 20:10 UTC by David
Modified: 2020-03-26 00:05 UTC (History)
5 users (show)

See Also:
koobs: mfc-stable11?
koobs: mfc-stable10?


Attachments
core.txt.1 (213.01 KB, application/octet-stream)
2013-11-04 19:38 UTC, Rod Taylor
no flags Details
info.1 (491 bytes, application/octet-stream)
2013-11-04 19:38 UTC, Rod Taylor
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description David 2013-07-27 20:10:00 UTC
rm -rf on a directory causes kernel panic and reboot.

panic: ufs_dirrem: Bad link count 2 on parent
cpuid = 0
KDB: stack backtrace:
#0 0xffffffff808680fe at kdb_backtrace+0x5e
#1 0xffffffff80832cb7 at panic+0x187
#2 0xffffffff80a700e3 at ufs_rmdir+0x1c3
#3 0xffffffff80b7d484 at VOP_RMDIR_APV+0x34
#4 0xffffffff808ca32a at kern_rmdirat+0x21a
#5 0xffffffff80b17cf0 at amd64_syscall+0x450
#6 0xffffffff80b03427 at Xfast_syscall+0xf7

I repeated this several times to be certain and it consistently blew up in my face so I'm guessing it's behaving as designed. It seems like there should be a better way to handle this than a kernel panic though. I've never crashed a box with "rm -rf" before. Might there be some alternative than the current design?
Comment 1 David 2013-07-28 06:20:58 UTC
From the freebsd-questions mailing list a solution was proposed by
Frank Leonhardt <frank2@fjl.co.uk>:

I was going to raise an issue when the discussion had died down to a
concensus. I also don't think it's reasonable for the kernel to bomb
when it encounters corruption on a disk.

If you want to patch it yourself, edit sys/ufs/ufs/ufs_vnops.c at
around line 2791 change:

        if (dp->i_effnlink < 3)
                panic("ufs_dirrem: Bad link count %d on parent",
                    dp->i_effnlink);

To

        if (dp->i_effnlink < 3) {
                error = EINVAL;
                goto out;
        }

The ufs_link() call has a similar issue.

I can't see why my mod will break anything, but there's always
unintended consequences. By returning invalid argument, any code above
it should already be handling that condition although the user will be
scratching their head wondering what's wrong with it. Returning ENOENT
or EACCES or ENOTDIR may be better ("No such directory", "Access
denied" or "Not a valid directory").

The trouble is that it's tricky to test properly without finding a
good way to corrupt the link count :-)

Regards, Frank.

----

Maybe something along these lines would be a reasonable fix?

-David
Comment 2 Peter Wemm freebsd_committer freebsd_triage 2013-07-28 19:08:59 UTC
Responsible Changed
From-To: freebsd-amd64->freebsd-bugs

Generic kernel bug, not amd64 specific.
Comment 3 Rod Taylor 2013-11-04 22:30:24 UTC
Instead of a panic when deleting the broken distinfo directory, I now get
the below:

[rbt@rbtlaptop /usr/ports/devel]$ sudo rm -rf p5-namespace-clean/
Password:
rm: p5-namespace-clean/distinfo: Invalid argument
rm: p5-namespace-clean/: Directory not empty

I expect a filesystem fsck will eventually repair this problem. In the mean
time, is there anything you would like me to do?
Comment 4 Kirk McKusick freebsd_committer freebsd_triage 2016-10-24 21:29:54 UTC
I'll take this bug.
Comment 5 commit-hook freebsd_committer freebsd_triage 2016-10-26 20:29:24 UTC
A commit references this bug:

Author: mckusick
Date: Wed Oct 26 20:28:23 UTC 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

Changes:
  head/sys/ufs/ufs/ufs_vnops.c
Comment 6 Kirk McKusick freebsd_committer freebsd_triage 2016-10-26 20:33:17 UTC
This fix will be MFC'ed to 11-stable and 10-stable in two weeks at which point this bug report will be closed.
Comment 7 Kubilay Kocak freebsd_committer freebsd_triage 2016-10-27 15:21:09 UTC
Assign to committer resolving, set mfc flags to reflect (comment 6) intent
Comment 8 commit-hook freebsd_committer freebsd_triage 2016-11-16 01:04:43 UTC
A commit references this bug:

Author: mckusick
Date: Wed Nov 16 01:03:42 UTC 2016
New revision: 308707
URL: https://svnweb.freebsd.org/changeset/base/308707

Log:
  MFC r307978:
  Bug 180894 reports that rm -rf on a directory causes kernel panic and reboot.
  Return EINVAL rather than panic for low directory link count.

  PR: 180894

Changes:
_U  stable/11/
  stable/11/sys/ufs/ufs/ufs_vnops.c
Comment 9 commit-hook freebsd_committer freebsd_triage 2016-11-22 00:27:35 UTC
A commit references this bug:

Author: mckusick
Date: Tue Nov 22 00:27:19 UTC 2016
New revision: 308946
URL: https://svnweb.freebsd.org/changeset/base/308946

Log:
  MFC r307978:
  Bug 180894 reports that rm -rf on a directory causes kernel panic and reboot.
  Return EINVAL rather than panic for low directory link count.

  PR: 180894

Changes:
_U  stable/10/
  stable/10/sys/ufs/ufs/ufs_vnops.c
Comment 10 Kirk McKusick freebsd_committer freebsd_triage 2016-11-22 00:36:49 UTC
The bugfix has been MFC'ed to 10-stable and 11-stable, so I am now closing it.