Re: Suspect code in fsck_ext2fs

[christos%astron.com@localhost (Christos Zoulas)]

In article <514B3EF3.6050003%gmail.com@localhost>,
Antoine LECA  <antoine.leca.1%gmail.com@localhost> wrote:
>-=-=-=-=-=-
>
>Hi guys,
>
>Hope this is the best list for that subject; do not doubt to redirect me
>to a better place though.
>
>While I am trying to chase a bug in my (non-NBSD) ext2fs code, I found
>what I suspect to be a problem in fsck_ext2fs, as the attached patch
>(1st) shows. Unfortunately due my own bug (that I did not solve) I am
>not able to confirm there needs to be fixed or not.
>
>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.
>
>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.)
>
>
>Antoine
>
>-=-=-=-=-=-
>
>commit 0c9092689f4cc3e67964c48bedfe5a57e61c3f3c
>Author: Antoine Leca <Antoine.Leca.1%gmail.com@localhost>
>Date:   Wed Mar 6 12:11:38 2013 +0000
>
>    fsck_ext2: fix e2d_type (?)
>
>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 /* ||
>


Please file a PR so it does not get lost.

christos