Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 25 Nov 2018 17:25:07 -0800
From:      Kirk McKusick <mckusick@mckusick.com>
To:        Rick Macklem <rmacklem@uoguelph.ca>
Cc:        "soralx@cydem.org" <soralx@cydem.org>, "freebsd-fs@freebsd.org" <freebsd-fs@freebsd.org>, "Julian H. Stacey" <jhs@berklix.com>, Konstantin Belousov <kostikbel@gmail.com>
Subject:   Re: [bug] fsck refuses to repair damaged UFS using backup superblock
Message-ID:  <201811260125.wAQ1P7uV059136@chez.mckusick.com>
In-Reply-To: <YTOPR0101MB1162D82F0211BF1FE7D6A4E1DDD60@YTOPR0101MB1162.CANPRD01.PROD.OUTLOOK.COM>

next in thread | previous in thread | raw e-mail | index | archive | help
> From: Rick Macklem <rmacklem@uoguelph.ca>
> To: Kirk McKusick <mckusick@mckusick.com>
> CC: "soralx@cydem.org" <soralx@cydem.org>,
>         "freebsd-fs@freebsd.org"
> 	<freebsd-fs@freebsd.org>,
>         "Julian H. Stacey" <jhs@berklix.com>,
>         Konstantin Belousov <kostikbel@gmail.com>
> Subject: Re: [bug] fsck refuses to repair damaged UFS using backup super=
block
> Thread-Topic: [bug] fsck refuses to repair damaged UFS using backup supe=
rblock
> Date: Mon, 26 Nov 2018 00:07:36 +0000
> =

> Rick Macklem wrote:
> [stuff snipped]
> Ok, it looks like the FS_METACKHASH flag was meant to handle the
> old->new kernel transition. However, if I am reading the code
> correctly, it needs to be checked for sooner. Here's the code
> snippet (from sys/ufs/fdfs/ffs_vfsops.c):
> =

>       /* fetch the superblock and summary information */
> 812        if ((error =3D ffs_sbget(devvp, &fs, -1, M_UFSMNT, ffs_use_br=
ead)) !=3D 0)
> 813                goto out;
> 814        fs->fs_fmod =3D 0;
> 815        /* if we ran on a kernel without metadata check hashes, disab=
le them */
> 816        if ((fs->fs_flags & FS_METACKHASH) =3D=3D 0)
> 817                fs->fs_metackhash =3D 0;
> =

> I think ffs_sbget() calls readsuper() which calls calc_sbhash(),
> so lines 815-817 need to be near the top of readsuper(), I think?
> =

> rick
> [more stuff snipped]

Quite right. I have moved the metadata check to just before the
superblock check-hash is done (in sys/ufs/ffs/ffs_subr.c) as you
suggested (see -r340927).

I wanted to ensure that running a filesystem on an older kernel
would be detected. I coopted the FS_INDEXDIRS bit (which the kernel
has always cleared since UFS2 was first released) to be the
FS_METACKHASH flag. Thus if that flag is cleared, none of the
check-hashes can be trusted. But I failed to make that check
early enough to protect the reading of the superblock. Now fixed.

All kernels since the first check-hashes were added no longer
clear the FS_METACKHASH flag, but do clear any of the specific
hashes that they do not support so that it will continue to be
safe to move filesystems between kernels as the check hashes are
added. When fsck_ffs is run in manual (non-preen) mode, it checks
for missing check-hashes supported by the currently running kernel
and offers to add them.

	Kirk McKusick



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201811260125.wAQ1P7uV059136>