Bug 263979 - [meta] UFS / FFS / GEOM crash (panic) tracking
Summary: [meta] UFS / FFS / GEOM crash (panic) tracking
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: Unspecified
Hardware: Any Any
: --- Affects Only Me
Assignee: Kirk McKusick
URL: https://reviews.freebsd.org/D35219
Keywords: crash, needs-qa, tracking
Depends on: 244342 244344 244346 244350 244351 244352 253617 257557 263765 263811 263822 263836 263934 263971
Blocks:
  Show dependency treegraph
 
Reported: 2022-05-15 02:30 UTC by Kubilay Kocak
Modified: 2022-12-04 05:21 UTC (History)
7 users (show)

See Also:
koobs: mfc-stable13?
koobs: mfc-stable12?


Attachments
GENERIC-BUG (14.90 KB, text/plain)
2022-11-21 16:39 UTC, Boris Korzun
no flags Details
proposed fix (555 bytes, patch)
2022-11-28 13:24 UTC, Kirk McKusick
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kubilay Kocak freebsd_committer freebsd_triage 2022-05-15 02:30:07 UTC
Tracking issue to make coordination of existing ffs/ufs crash reports easier. Linked issues involve at least some aspect of ffs in the crash backtraces
Comment 1 Graham Perrin freebsd_committer freebsd_triage 2022-05-15 10:06:17 UTC
See also: bug 244384
Comment 2 Kirk McKusick freebsd_committer freebsd_triage 2022-05-15 18:50:55 UTC
(In reply to Kubilay Kocak from comment #0)
Thanks for pulling these all together in a single place.

I am working on a general fix that should cover all of these bugs. The fix verifies the superblock when it is read in. Since there is a single piece of code that is used to read the superblock that is shared by the kernel, boot, and user code, it should cover all of these bugs. Stay tuned for a phabricator review.
Comment 3 Kirk McKusick freebsd_committer freebsd_triage 2022-05-16 00:08:53 UTC
See https://reviews.freebsd.org/D35219 for a proposed fix to these bug reports.
Comment 4 Kubilay Kocak freebsd_committer freebsd_triage 2022-05-17 08:40:47 UTC
(In reply to Kirk McKusick from comment #2)

My pleasure Kirk. I believe a couple/few look like dupes, I'll be normalizing the summaries to make those clearer and close any dupes (with the older remaining open) tomorrow
Comment 5 Kubilay Kocak freebsd_committer freebsd_triage 2022-05-17 08:43:54 UTC
P.S If this ends up being the issue in which analysis, comment and patches take place, rather than only a tracking issue, with resolution taking place separately for each existing issue, I'll switch the dependency to 'Blocks' rather than the current 'Depends on'.

^Triage: Kirks coordinating here (comment 2, assign accordingly)
Comment 6 commit-hook freebsd_committer freebsd_triage 2022-05-27 19:23:24 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=076002f24d35962f0d21f44bfddd34ee4d7f015d

commit 076002f24d35962f0d21f44bfddd34ee4d7f015d
Author:     Kirk McKusick <mckusick@FreeBSD.org>
AuthorDate: 2022-05-27 19:21:11 +0000
Commit:     Kirk McKusick <mckusick@FreeBSD.org>
CommitDate: 2022-05-27 19:22:07 +0000

    Do comprehensive UFS/FFS superblock integrity checks when reading a superblock.

    Historically only minimal checks were made of a superblock when it
    was read in as it was assumed that fsck would have been run to
    correct any errors before attempting to use the filesystem. Recently
    several bug reports have been submitted reporting kernel panics
    that can be triggered by deliberately corrupting filesystem superblocks,
    see Bug 263979 - [meta] UFS / FFS / GEOM crash (panic) tracking
    which is tracking the reported corruption bugs.

    This change upgrades the checks that are performed. These additional
    checks should prevent panics from a corrupted superblock. Although
    it appears in only one place, the new code will apply to the kernel
    modules and (through libufs) user applications that read in superblocks.

    Reported by:  Robert Morris and Neeraj
    Reviewed by:  kib
    Tested by:    Peter Holm
    PR:           263979
    MFC after:    1 month
    Differential Revision: https://reviews.freebsd.org/D35219

 sys/ufs/ffs/ffs_subr.c | 163 +++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 146 insertions(+), 17 deletions(-)
Comment 7 commit-hook freebsd_committer freebsd_triage 2022-11-18 01:22:25 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=b999366aab4e2d59cb8869b0e5ef0f70ab9b9bbe

commit b999366aab4e2d59cb8869b0e5ef0f70ab9b9bbe
Author:     Kirk McKusick <mckusick@FreeBSD.org>
AuthorDate: 2022-05-27 19:21:11 +0000
Commit:     Kirk McKusick <mckusick@FreeBSD.org>
CommitDate: 2022-11-18 01:19:41 +0000

    Do comprehensive UFS/FFS superblock integrity checks when reading a superblock.

    Historically only minimal checks were made of a superblock when it
    was read in as it was assumed that fsck would have been run to
    correct any errors before attempting to use the filesystem. Recently
    several bug reports have been submitted reporting kernel panics
    that can be triggered by deliberately corrupting filesystem superblocks,
    see Bug 263979 - [meta] UFS / FFS / GEOM crash (panic) tracking
    which is tracking the reported corruption bugs.

    This change upgrades the checks that are performed. These additional
    checks should prevent panics from a corrupted superblock. Although
    it appears in only one place, the new code will apply to the kernel
    modules and (through libufs) user applications that read in superblocks.

    Reported by:  Robert Morris and Neeraj
    Reviewed by:  kib
    Tested by:    Peter Holm
    PR:           263979
    Differential Revision: https://reviews.freebsd.org/D35219

    (cherry picked from commit 076002f24d35962f0d21f44bfddd34ee4d7f015d)
    (cherry picked from commit bc218d89200faa021def77732f3d9fde4f4dee13)
    (cherry picked from commit 800a53b445e7eb113ba193b1ac98631299178529)
    (cherry picked from commit 50dc4c7df4156863148e6a9609c03e852e2aeb35)
    (cherry picked from commit f3f5368dfbef4514686ba2d67f01f314b275227e)
    (cherry picked from commit 9e1f44d044a58fcd2caaca3f57e69cf6180db3dc)
    (cherry picked from commit 5bc926af9fd1c47f74356734f731c68145e31c6f)
    (cherry picked from commit 904347a00c1f9a29f3b17e6e676805036d2494f1)
    (cherry picked from commit 36e08b0127f97928a2f2c062feed8df9087b2b35)
    (cherry picked from commit 548045bf57c46cb2f4d43d3d7fa5d8ad37ec7f9a)
    (cherry picked from commit 3e40d2cc61a00a7d69e99b0fda4040cd1df04c57)
    (cherry picked from commit 184e3118c1057a97e16230baf0f0433adeeed723)
    (cherry picked from commit f0be378a66a75ebf335e9388ef0d319a70064d94)
    (cherry picked from commit 9dee5da7450e8530c9fec51c9a16ecd42da78e55)
    (cherry picked from commit 82ee4e1c42d70345cbaa1f6dd1874ae98a004910)
    (cherry picked from commit dcdba3460dd779a0180ec7769ab8cd47c932799e)
    (cherry picked from commit 017367c1146a69baca6a1a0bea10b0cb02c72d85)
    (cherry picked from commit 8435a9b20684ba8bcda3df31d06b4d5eac9431a7)

 sys/ufs/ffs/ffs_subr.c | 303 +++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 257 insertions(+), 46 deletions(-)
Comment 8 Boris Korzun 2022-11-18 14:49:38 UTC
There'is a regression after b999366aab4e2d59cb8869b0e5ef0f70ab9b9bbe.
buildkernel falls on amd64:

--- kernel.full ---
ld: error: undefined symbol: sysctl___vfs_ffs
>>> referenced by ffs_subr.c
>>>               ffs_subr.o:(sysctl___vfs_ffs_prtsberrmsg)
Comment 9 Mark Millard 2022-11-18 18:22:16 UTC
(In reply to Boris Korzun from comment #8)

FYI:

https://lists.freebsd.org/archives/dev-commits-src-branches/2022-November/007958.html

reports:

From: Kirk McKusick <mckusick_at_FreeBSD.org>
Date: Fri, 18 Nov 2022 05:06:50 UTC 
The branch stable/13 has been updated by mckusick:

URL: https://cgit.FreeBSD.org/src/commit/?id=c2a74ca2c3d9ad3465f7cb88ea60907a097fbdbc

commit c2a74ca2c3d9ad3465f7cb88ea60907a097fbdbc
Author:     Kirk McKusick <mckusick@FreeBSD.org>
AuthorDate: 2022-11-18 05:03:01 +0000
Commit:     Kirk McKusick <mckusick@FreeBSD.org>
CommitDate: 2022-11-18 05:06:16 +0000

    Fix to b999366aab4e for compilation on i386.
    
    Reported by: jenkins, Philip Paeps
. . .
Comment 10 Kirk McKusick freebsd_committer freebsd_triage 2022-11-18 23:47:02 UTC
This class of bugs has been fixed in 14 as detailed in https://reviews.freebsd.org/D35219

MFC'ed to 13 with commit b999366aab4e2d59cb8869b0e5ef0f70ab9b9bbe on Fri May 27 12:21:11 2022 -0700

Too old in 12 life to be candidate for MFC.
Comment 11 Boris Korzun 2022-11-21 10:33:00 UTC
(In reply to Mark Millard from comment #9)
The problem is still reproducible after the c2a74ca2c3d9ad3465f7cb88ea60907a097fbdbc
Comment 12 Mark Millard 2022-11-21 11:19:28 UTC
(In reply to Boris Korzun from comment #11)

https://ci.freebsd.org/job/FreeBSD-stable-13-amd64-build/

reports for the official builds on the build servers:

Last build (#2244), 2 hr 5 min ago
Last stable build (#2244), 2 hr 5 min ago
Last successful build (#2244), 2 hr 5 min ago
Last failed build (#2237), 3 days 6 hr ago
Last unstable build (#1532), 8 mo 29 days ago
Last unsuccessful build (#2237), 3 days 6 hr ago
Last completed build (#2244), 2 hr 5 min ago

The successful builds since that last failure are of
(increasing time):

c2a74ca2c3d9ad3465f7cb88ea60907a097fbdbc (the fix mentioned in #9)
d3b97a1ea01233486ddc7693e8a53c59d331c8c2 (more recent commits)
d878a66a9a77fc67801000bfab8fa77b2b3faecc
689a5b194435e93baeda7cd132f2a6d1d8fe88eb
640799e5f2d0b09b359ce040bf7cbbcb6f33393d
bca262f0f351e3881d478b7a9764c91ccb83c260
7a5de4db8cf1862673f18b8b3b3d524c3847e51e

I expect a problem in your environment.
Comment 13 Boris Korzun 2022-11-21 13:10:17 UTC
(In reply to Mark Millard from comment #12)

Yep, I don't use an FFS option in my KERNEL.
=====
options 	FFS			# Berkeley Fast Filesystem
=====

The buildkernel process is ok with this option.
Comment 14 Kirk McKusick freebsd_committer freebsd_triage 2022-11-21 16:22:40 UTC
(In reply to Boris Korzun from comment #13)
I just built a 14 kernel without the FFS option and it worked fine. I will try building a 13 kernel without FFS to see if I can reproduce your build failure.
Comment 15 Kirk McKusick freebsd_committer freebsd_triage 2022-11-21 16:33:44 UTC
(In reply to Boris Korzun from comment #13)
My 13 build was successful. Can you please provide you configuration file so I can try building with it.
Comment 16 Boris Korzun 2022-11-21 16:39:56 UTC
Created attachment 238218 [details]
GENERIC-BUG

(In reply to Kirk McKusick from comment #15)

Only commented FFS options from GENERIC.
Comment 17 Kirk McKusick freebsd_committer freebsd_triage 2022-11-21 16:47:35 UTC
(In reply to Boris Korzun from comment #16)
Just commenting out FFS from GENERIC builds fine for me. Can you send me the output of your build that shows the error?
Comment 18 Mark Millard 2022-11-21 18:27:56 UTC
(In reply to Boris Korzun from comment #16)

src.conf ? make.conf ? Anything else that might contribute to your
buildkernel details?
Comment 19 Mark Millard 2022-11-21 18:38:32 UTC
(In reply to Mark Millard from comment #18)

Cross checks on work tree status? :

git log --oneline --no-color -n 1
git branch --show-current
git status
Comment 20 Boris Korzun 2022-11-21 21:39:19 UTC
(In reply to Mark Millard from comment #18)

There're no src.conf and make.conf.

root@boris:/usr/src# git log --oneline --no-color -n 1
c2a74ca2c3 (HEAD -> stable/13, origin/stable/13) Fix to b999366aab4e for compilation on i386.
root@boris:/usr/src# git branch --show-current
stable/13
root@boris:/usr/src# git status
On branch stable/13
Your branch is up to date with 'origin/stable/13'.

Untracked files:
  (use "git add <file>..." to include in what will be committed)
        sys/amd64/conf/GENERIC-BUG

nothing added to commit but untracked files present (use "git add" to track)
Comment 21 Boris Korzun 2022-11-26 21:55:02 UTC
(In reply to Kirk McKusick from comment #17)

I've downloaded FreeBSD-13.1-STABLE-amd64-20221123-b51ee7ac252c-253133-bootonly.iso, installed it on clean ZFS system.

Tried to buildkernel with commented FFS options from GENERIC only. And got:

ld: error: undefined symbol: sysctl___vfs_ffs
>>> referenced by ffs_subr.c
>>>               ffs_subr.o:(sysctl___vfs_ffs_prtsberrmsg)
>>> did you mean: sysctl___vfs_nfs
>>> defined in: nfs_commonport.o
*** Error code 1

124c19b0d (HEAD -> stable/13, freebsd/stable/13) amd64 libc: add missed GNU-stack annotation to memmove/memcpy
Comment 22 Mark Millard 2022-11-27 01:18:29 UTC
(In reply to Boris Korzun from comment #21)

So, I info that for:

#ifdef STANDALONE_SMALL
#define MPRINT(...)	do { } while (0)
#else
#define MPRINT(...)	if (prtmsg) printf(__VA_ARGS__)
/*
 * Print error messages when bad superblock values are found.
 */
static int prtmsg = 1;
#ifdef _KERNEL
SYSCTL_DECL(_vfs_ffs);
SYSCTL_INT(_vfs_ffs, OID_AUTO, prtsberrmsg, CTLFLAG_RWTUN, &prtmsg, 0,
    "Print error messages when bad superblock values are found");
#endif /* _KERNEL */
#endif /* STANDALONE_SMALL */

both STANDALONE_SMALL and _KERNEL are defined, lead to
the sysctl involved. But also:

sys/ufs/ffs/ffs_subr.c

is being compiled and linked in order for that to even matter.
Comment 23 Mark Millard 2022-11-27 01:52:06 UTC
(In reply to Mark Millard from comment #22)

May be the answer is geom_label not being disabled as well?

ufs/ffs/ffs_subr.c              optional ffs | geom_label
Comment 24 Boris Korzun 2022-11-28 09:11:28 UTC
(In reply to Mark Millard from comment #23)
Yep, the kernel is built successful without FFS and GEOM_LABEL options.

But there wasn't a problem before b999366aab4e2d59cb8869b0e5ef0f70ab9b9bbe with GEOM_LABEL option and without FFS option.
Comment 25 Kirk McKusick freebsd_committer freebsd_triage 2022-11-28 13:24:43 UTC
Created attachment 238401 [details]
proposed fix
Comment 26 Boris Korzun 2022-11-28 19:11:37 UTC
(In reply to Kirk McKusick from comment #25)
Thx. The fix is working properly.
Comment 27 commit-hook freebsd_committer freebsd_triage 2022-12-04 05:21:55 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=896c3e6e8a61606adf4414279c4699918f921510

commit 896c3e6e8a61606adf4414279c4699918f921510
Author:     Kirk McKusick <mckusick@FreeBSD.org>
AuthorDate: 2022-12-04 05:20:39 +0000
Commit:     Kirk McKusick <mckusick@FreeBSD.org>
CommitDate: 2022-12-04 05:21:32 +0000

    Fix compilation issue when kernel built without FFS defined.

    This is not cherry-picked because it occurs only in 13 and not
    in 14 or later kernels.

    Reported by:  Boris Korzun
    Tested by:    Boris Korzun
    PR:           263979
    Sponsored by: The FreeBSD Foundation

 sys/ufs/ffs/ffs_subr.c | 2 ++
 1 file changed, 2 insertions(+)