NetBSD Problem Report #58876

From www@netbsd.org  Thu Dec  5 16:38:48 2024
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
	 key-exchange X25519 server-signature RSA-PSS (2048 bits)
	 client-signature RSA-PSS (2048 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id B70741A9238
	for <gnats-bugs@gnats.NetBSD.org>; Thu,  5 Dec 2024 16:38:48 +0000 (UTC)
Message-Id: <20241205163847.2CBD11A923B@mollari.NetBSD.org>
Date: Thu,  5 Dec 2024 16:38:47 +0000 (UTC)
From: jlduran@gmail.com
Reply-To: jlduran@gmail.com
To: gnats-bugs@NetBSD.org
Subject: mtree: Produces a different checksum when verifying
X-Send-Pr-Version: www-1.0

>Number:         58876
>Category:       bin
>Synopsis:       mtree: Produces a different checksum when verifying
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    bin-bug-people
>State:          needs-pullups
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Dec 05 16:40:02 +0000 2024
>Closed-Date:    
>Last-Modified:  Thu Dec 05 19:16:16 +0000 2024
>Originator:     Jose Luis Duran
>Release:        Not applicable to NetBSD
>Organization:
FreeBSD
>Environment:
NOTE: This bug is not present on NetBSD, only on FreeBSD. FreeBSD uses mtree from NetBSD.
>Description:
A bug report was opened[^1] stating that a user following the procedure described in the FreeBSD handbook[^2] to generate and verify an mtree-based specification file produced non-matching checksums. Everything is working fine, except that the output produces two different checksums.

Prior to FreeBSD using NetBSD's mtree, a commit fixed this issue[^3]. This fix is not present on NetBSD, as the issue is not manifested on NetBSD.

[^1]: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=231072
[^2]: https://docs.freebsd.org/en/books/handbook/security/#security-ids-generate-spec-file
[^3]: https://cgit.freebsd.org/src/commit/?id=afe54b5818317b574a1b4b0f88c1643efc2c8da2
>How-To-Repeat:
On FreeBSD:

# mtree -s 123456789 -c -K cksum,sha512 -p /bin > /home/user/.bin_chksum_mtree
mtree: /bin checksum: 1461382254
# mtree -s 123456789 -p /bin < /home/user/.bin_chksum_mtree >> /home/user/.bin_chksum_output
mtree: /bin checksum: 3158826036

The resulting file is empty (there is no mismatch), however the checksum is different.

>Fix:
Use a fix similar to the one proposed in the pull request, rather than the original FreeBSD fix, as I *think* it blends better with the current code, but feel free to re-arrange it as you see fit.

The idea is to traverse the existing directory tree in lexical order, to allow a direct comparison between different runs regardless of the order of what readdir(3) returns.

Move dcmp() next to nodecmp(), as these two functions go hand-in-hand, and pass it as the compar() argument to fts_open(3) in vwalk(), just like in cwalk().

The proposed patch is attached:

Subject: [PATCH] mtree: Traverse in lexical order

When applying a spec, traverse the existing directory tree in lexical
order.  This allows a direct comparison between the output of two
different runs, regardless of the order in which readdir(3) returns
directory entries.

FreeBSD PR:	231072
---
 usr.sbin/mtree/create.c | 28 ----------------------------
 usr.sbin/mtree/extern.h |  7 +++++++
 usr.sbin/mtree/spec.c   | 21 +++++++++++++++++++++
 usr.sbin/mtree/verify.c |  2 +-
 4 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/usr.sbin/mtree/create.c b/usr.sbin/mtree/create.c
index bb5d2f21ad80..4962b51050ec 100644
--- a/usr.sbin/mtree/create.c
+++ b/usr.sbin/mtree/create.c
@@ -84,13 +84,6 @@ static uid_t uid;
 static mode_t mode;
 static u_long flags;

-#if defined(__FreeBSD__) && !defined(HAVE_NBTOOL_CONFIG_H)
-#define	FTS_CONST const
-#else
-#define	FTS_CONST
-#endif
-
-static int	dcmp(const FTSENT *FTS_CONST *, const FTSENT *FTS_CONST *);
 static void	output(FILE *, int, int *, const char *, ...)
     __printflike(4, 5);
 static int	statd(FILE *, FTS *, FTSENT *, uid_t *, gid_t *, mode_t *,
@@ -444,27 +437,6 @@ statd(FILE *fp, FTS *t, FTSENT *parent, uid_t *puid, gid_t *pgid, mode_t *pmode,
 	return (0);
 }

-/*
- * dcmp --
- *	used as a comparison function passed to fts_open() to control
- *	the order in which fts_read() returns results.	We make
- *	directories sort after non-directories, but otherwise sort in
- *	strcmp() order.
- *
- * Keep this in sync with nodecmp() in spec.c.
- */
-static int
-dcmp(const FTSENT *FTS_CONST *a, const FTSENT *FTS_CONST *b)
-{
-
-	if (S_ISDIR((*a)->fts_statp->st_mode)) {
-		if (!S_ISDIR((*b)->fts_statp->st_mode))
-			return (1);
-	} else if (S_ISDIR((*b)->fts_statp->st_mode))
-		return (-1);
-	return (strcmp((*a)->fts_name, (*b)->fts_name));
-}
-
 void
 output(FILE *fp, int indent, int *offset, const char *fmt, ...)
 {
diff --git a/usr.sbin/mtree/extern.h b/usr.sbin/mtree/extern.h
index da34e99f1a9a..bb82bd19f8bd 100644
--- a/usr.sbin/mtree/extern.h
+++ b/usr.sbin/mtree/extern.h
@@ -49,6 +49,12 @@
 #include <netdb.h>
 #endif

+#if defined(__FreeBSD__) && !defined(HAVE_NBTOOL_CONFIG_H)
+#define	FTS_CONST const
+#else
+#define	FTS_CONST
+#endif
+
 #ifndef MAXHOSTNAMELEN
 #define MAXHOSTNAMELEN 256
 #endif
@@ -64,6 +70,7 @@ int	 check_excludes(const char *, const char *);
 int	 compare(NODE *, FTSENT *);
 int	 crc(int, uint32_t *, uint32_t *);
 void	 cwalk(FILE *);
+int	 dcmp(const FTSENT *FTS_CONST *, const FTSENT *FTS_CONST *);
 void	 dump_nodes(FILE *, const char *, NODE *, int);
 void	 init_excludes(void);
 int	 matchtags(NODE *);
diff --git a/usr.sbin/mtree/spec.c b/usr.sbin/mtree/spec.c
index 25eac4b55ade..045fcea55ccb 100644
--- a/usr.sbin/mtree/spec.c
+++ b/usr.sbin/mtree/spec.c
@@ -855,3 +855,24 @@ nodecmp(const NODE *a, const NODE *b)
 		return -1;
 	return strcmp(a->name, b->name);
 }
+
+/*
+ * dcmp --
+ *	used as a comparison function passed to fts_open() to control
+ *	the order in which fts_read() returns results.	We make
+ *	directories sort after non-directories, but otherwise sort in
+ *	strcmp() order.
+ *
+ * Keep this in sync with nodecmp() above.
+ */
+int
+dcmp(const FTSENT *FTS_CONST *a, const FTSENT *FTS_CONST *b)
+{
+
+	if (S_ISDIR((*a)->fts_statp->st_mode)) {
+		if (!S_ISDIR((*b)->fts_statp->st_mode))
+			return (1);
+	} else if (S_ISDIR((*b)->fts_statp->st_mode))
+		return (-1);
+	return (strcmp((*a)->fts_name, (*b)->fts_name));
+}
diff --git a/usr.sbin/mtree/verify.c b/usr.sbin/mtree/verify.c
index 2430fb1b8066..520a329006ba 100644
--- a/usr.sbin/mtree/verify.c
+++ b/usr.sbin/mtree/verify.c
@@ -86,7 +86,7 @@ vwalk(void)
 	argv[0] = dot;
 	argv[1] = NULL;

-	if ((t = fts_open(argv, ftsoptions, NULL)) == NULL)
+	if ((t = fts_open(argv, ftsoptions, dcmp)) == NULL)
 		mtree_err("fts_open: %s", strerror(errno));
 	level = root;
 	specdepth = rval = 0;
-- 
Jose Luis Duran

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58875 CVS commit: src/usr.sbin/mtree
Date: Thu, 5 Dec 2024 12:17:43 -0500

 Module Name:	src
 Committed By:	christos
 Date:		Thu Dec  5 17:17:43 UTC 2024

 Modified Files:
 	src/usr.sbin/mtree: compare.c crc.c create.c extern.h misc.c spec.c
 	    verify.c

 Log Message:
 PR/58875: Jose Luis Duran: Produce consistent checksums in verification
 by scanning directories in the same order as usual. While here, fix some
 incorrect types.


 To generate a diff of this commit:
 cvs rdiff -u -r1.60 -r1.61 src/usr.sbin/mtree/compare.c
 cvs rdiff -u -r1.10 -r1.11 src/usr.sbin/mtree/crc.c
 cvs rdiff -u -r1.78 -r1.79 src/usr.sbin/mtree/create.c
 cvs rdiff -u -r1.40 -r1.41 src/usr.sbin/mtree/extern.h
 cvs rdiff -u -r1.34 -r1.35 src/usr.sbin/mtree/misc.c
 cvs rdiff -u -r1.91 -r1.92 src/usr.sbin/mtree/spec.c
 cvs rdiff -u -r1.48 -r1.49 src/usr.sbin/mtree/verify.c

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

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58875 CVS commit: src/usr.sbin/mtree
Date: Thu, 5 Dec 2024 12:17:15 -0500

 Module Name:	src
 Committed By:	christos
 Date:		Thu Dec  5 17:17:15 UTC 2024

 Modified Files:
 	src/usr.sbin/mtree: mtree.c

 Log Message:
 PR/58875: Jose Luis Duran: Produce consistent checksums in verification
 by scanning directories in the same order as usual. While here, fix some
 incorrect types.


 To generate a diff of this commit:
 cvs rdiff -u -r1.50 -r1.51 src/usr.sbin/mtree/mtree.c

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


>Release-Note:

>Audit-Trail:

State-Changed-From-To: open->closed
State-Changed-By: christos@NetBSD.org
State-Changed-When: Thu, 05 Dec 2024 13:11:52 -0500
State-Changed-Why:
fixed.


State-Changed-From-To: closed->needs-pullups
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Thu, 05 Dec 2024 19:16:16 +0000
State-Changed-Why:
probably worth pulling up to 9 and 10


>Unformatted:

Home	PR Database Search