NetBSD Problem Report #44209

From riz@netbsd6.localdomain  Thu Dec  9 00:04:40 2010
Return-Path: <riz@netbsd6.localdomain>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id A219F63B87A
	for <gnats-bugs@gnats.NetBSD.org>; Thu,  9 Dec 2010 00:04:40 +0000 (UTC)
Message-Id: <20101209000439.E1C4D1BF45F@netbsd6.localdomain>
Date: Wed,  8 Dec 2010 16:04:39 -0800 (PST)
From: riz@netbsd.org
Reply-To: riz@netbsd.org
To: gnats-bugs@gnats.NetBSD.org
Subject: superblock check can fail erroneously in resize_ffs(8)
X-Send-Pr-Version: 3.95

>Number:         44209
>Category:       bin
>Synopsis:       superblock check can fail erroneously in resize_ffs(8)
>Confidential:   no
>Severity:       non-critical
>Priority:       high
>Responsible:    bin-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Dec 09 00:05:00 +0000 2010
>Closed-Date:    Tue Dec 21 06:05:40 +0000 2010
>Last-Modified:  Tue Dec 21 06:05:40 +0000 2010
>Originator:     Jeff Rizzo <riz@NetBSD.org>
>Release:        NetBSD 5.99.41
>Organization:

>Environment:


System: NetBSD netbsd6 5.99.41 NetBSD 5.99.41 (GENERIC) #0: Sun Dec 5 19:35:40 PST 2010 riz@netbsd6:/home/riz/obj/sys/arch/amd64/compile/GENERIC amd64
Architecture: x86_64
Machine: amd64
>Description:
	The superblock search code in resize_ffs(8) is looking
for the wrong things, and can falsely claim not to handle a particular
superblock.  Also, it's not ffsv2 format blocks it has trouble with,
but actually ffsv1.

This code in resize_ffs.c main() is the culprit:

	for (where = search[i = 0]; search[i] != -1; where = search[++i]) {
		readat(where / DEV_BSIZE, oldsb, SBLOCKSIZE);
		if (where == SBLOCK_UFS2 && (oldsb->fs_magic == FS_UFS1_MAGIC))
			continue;
		if (oldsb->fs_magic == FS_UFS1_MAGIC)
			break;
		if (oldsb->fs_old_flags & FS_FLAGS_UPDATED)
			err(EXIT_FAILURE,
			    "Can't resize ffsv2 format superblock!");
	}

It is sometimes possible to hit the error condition on an otherwise-valid
file system.  The check should really be more like:

	if ((oldsb->fs_old_flags & FS_FLAGS_UPDATED) == 0)
		errx(EXIT_FAILURE, "Can't resize v1 format superblock!\n"
		    "Consider updating to a v2 superblock with "
		    "fsck_ffs -c 4.");

>How-To-Repeat:
	Try to resize a filesystem with just the wrong data.
>Fix:
	I have a fix in my local tree;  there are some more extensive changes
that will be made at the same time, so I'm holding off for a few days.


This PR is filed primarily because I will likely commit an expected-fail test
prior to fixing this PR.

>Release-Note:

>Audit-Trail:
From: "Michael L. Hitch" <mhitch@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/44209 CVS commit: src/sbin/resize_ffs
Date: Sun, 12 Dec 2010 19:53:23 +0000

 Module Name:	src
 Committed By:	mhitch
 Date:		Sun Dec 12 19:53:23 UTC 2010

 Modified Files:
 	src/sbin/resize_ffs: resize_ffs.c

 Log Message:
 Add support for old ffsv1 superblocks.  After reading an old superblock,
 copy appropriate data to where they are expected in the updated superblock.
 When writing the updated superblock, move the updated values back to the
 old ffsv1 superblock locations.  Also check for old superblock format when
 updating the last cylinder group and adjust cg_old_ncyl appropriately.
 Derived from how mksf sets them.  Should address PR bin/44209.


 To generate a diff of this commit:
 cvs rdiff -u -r1.19 -r1.20 src/sbin/resize_ffs/resize_ffs.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Michael L. Hitch" <mhitch@lightning.msu.montana.edu>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: PR/44209 CVS commit: src/sbin/resize_ffs
Date: Sun, 12 Dec 2010 14:22:25 -0700 (MST)

 On Sun, 12 Dec 2010, Michael L. Hitch wrote:

 > Modified Files:
 > 	src/sbin/resize_ffs: resize_ffs.c
 >
 > Log Message:
 > Add support for old ffsv1 superblocks.  After reading an old superblock,
 > copy appropriate data to where they are expected in the updated superblock.
 > When writing the updated superblock, move the updated values back to the
 > old ffsv1 superblock locations.  Also check for old superblock format when
 > updating the last cylinder group and adjust cg_old_ncyl appropriately.
 > Derived from how mksf sets them.  Should address PR bin/44209.

    I misunderstood the problem described in this PR, so my changes don't 
 have anything to do with the PR.

    But I do think I know what is causing the error message to be hit.  The 
 first attempt to find the superblock looks for a superblock at offset 64K, 
 which is where the superblock for a ffsv2 file system resides.  The magic 
 won't match and it falls through to the test of the fs_old_flags.  If the 
 random data at that point has the flag bit set, the error will trigger. 
 So that check is completely bogus and should just be removed.  In fact, 
 the message says that it can't resize an ffsv2 format superblock, but 
 that's the only format superblock that resize_ffs.c could handle (but only
 for an ffsv1 filesystem).  My commit fixes it so it can handle either
 an ffsv1 format superblock kor an ffsv2 format superblock on an ffsv1
 file system.

 --
 Michael L. Hitch			mhitch@montana.edu
 Computer Consultant
 Information Technology Center
 Montana State University	Bozeman, MT	USA

From: "Jeff Rizzo" <riz@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/44209 CVS commit: src/tests/sbin/resize_ffs
Date: Sun, 12 Dec 2010 22:50:42 +0000

 Module Name:	src
 Committed By:	riz
 Date:		Sun Dec 12 22:50:42 UTC 2010

 Modified Files:
 	src/tests/sbin/resize_ffs: t_grow.sh t_shrink.sh

 Log Message:
 PR bin/44209 is now fixed, expect failure -> expect success.


 To generate a diff of this commit:
 cvs rdiff -u -r1.3 -r1.4 src/tests/sbin/resize_ffs/t_grow.sh \
     src/tests/sbin/resize_ffs/t_shrink.sh

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Tue, 21 Dec 2010 06:05:40 +0000
State-Changed-Why:
if it's fixed please also close the PR :-)


>Unformatted:

Home	PR Database Search