NetBSD Problem Report #47681

From www@NetBSD.org  Fri Mar 22 08:01:45 2013
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	by www.NetBSD.org (Postfix) with ESMTP id A8A7263F15B
	for <gnats-bugs@gnats.NetBSD.org>; Fri, 22 Mar 2013 08:01:45 +0000 (UTC)
Message-Id: <20130322080144.7C6EC63F15B@www.NetBSD.org>
Date: Fri, 22 Mar 2013 08:01:44 +0000 (UTC)
From: Antoine.Leca.1@gmail.com
Reply-To: Antoine.Leca.1@gmail.com
To: gnats-bugs@NetBSD.org
Subject: Suspect code in fsck_ext2fs
X-Send-Pr-Version: www-1.0

>Number:         47681
>Category:       bin
>Synopsis:       Suspect code in fsck_ext2fs
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Mar 22 08:05:00 +0000 2013
>Originator:     Antoine Leca
>Release:        NetBSD 6.0
>Organization:
>Environment:
System: NetBSD netbsd6.local 6.0 NetBSD 6.0 (GENERIC) amd64
>Description:
I found what I suspect to be a problem in fsck_ext2fs/dir.c. My understanding is that in that function dircheck(), returning (1) means all_is_good, while (0) indicates some problem; also, I understand a non-zero e2d_type in some entry while either using Rev.0 or with the INCOMPAT_FTYPE unset, is NOT a normal condition.

Unfortunately due my own bugs (in foreign code) I am not able to confirm whether it needs to be fixed or not.

While here, I realized that the actual value for e2d_type was not checked in the case of Rev.1 FS, so I designed another patch (2nd) to add that check. No doubt it can be better written!

Perhaps related is that, when the function returns(0) while there are dirty values within the directory entries, then fsck stops on that directory and proposes to "Salvage" the directory, which I read to be a pretty drastic operation, dropping all the entries after the faulty one if I understand correctly (I am all of a newbie to FFS and derivatives, so do not hesitate to correct me where I am wrong.)
>How-To-Repeat:
Take some (dispensable) ext2fs file system.
Change the e2d_type byte of a directory entry.
Then run fsck_ext2fs -f against that file system.
>Fix:
Obvious fix:

diff --git a/sbin/fsck_ext2fs/dir.c b/sbin/fsck_ext2fs/dir.c
index 8036f52..66d36ec 100644
--- a/sbin/fsck_ext2fs/dir.c
+++ b/sbin/fsck_ext2fs/dir.c
@@ -274,7 +274,7 @@ dircheck(struct inodesc *idesc, struct ext2fs_direct *dp)
 	if (sblock.e2fs.e2fs_rev < E2FS_REV1 ||
 	    (sblock.e2fs.e2fs_features_incompat & EXT2F_INCOMPAT_FTYPE) == 0)
 		if (dp->e2d_type != 0)
-			return (1);
+			return (0);
 	size = EXT2FS_DIRSIZ(dp->e2d_namlen);
 	if (reclen < size ||
 	    idesc->id_filesize < size /* || 



Further improvement of the utility:
diff --git a/sbin/fsck_ext2fs/dir.c b/sbin/fsck_ext2fs/dir.c
index 66d36ec..f6327eb 100644
--- a/sbin/fsck_ext2fs/dir.c
+++ b/sbin/fsck_ext2fs/dir.c
@@ -272,9 +272,13 @@ dircheck(struct inodesc *idesc, struct ext2fs_direct *dp)
 	if (dp->e2d_ino == 0)
 		return (1);
 	if (sblock.e2fs.e2fs_rev < E2FS_REV1 ||
-	    (sblock.e2fs.e2fs_features_incompat & EXT2F_INCOMPAT_FTYPE) == 0)
+	    (sblock.e2fs.e2fs_features_incompat & EXT2F_INCOMPAT_FTYPE) == 0) {
 		if (dp->e2d_type != 0)
 			return (0);
+	} else {
+		if (dp->e2d_type >= EXT2_FT_MAX)
+			return (0);
+	}
 	size = EXT2FS_DIRSIZ(dp->e2d_namlen);
 	if (reclen < size ||
 	    idesc->id_filesize < size /* || 

Home	PR Database Search