lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 14 Jan 2020 14:42:17 -0700
From:   Andreas Dilger <adilger@...ger.ca>
To:     tytso@....edu
Cc:     linux-ext4@...r.kernel.org, Andreas Dilger <adilger@...ger.ca>
Subject: [PATCH 1/2] mmp: don't assume NUL termination for MMP strings

Don't assume that mmp_nodename and mmp_bdevname are NUL terminated,
since very long node/device names may completely fill the buffers.

Limit string printing to the maximum buffer size for safety, and
change the field definitions to __u8 to make it more clear that
they are not NUL-terminated strings, as is done with other strings
in the superblock that do not have NUL termination.

Signed-off-by: Andreas Dilger <adilger@...ger.ca>
---
 debugfs/debugfs.c    |  6 ++++--
 e2fsck/util.c        |  8 ++++++--
 lib/ext2fs/ext2_fs.h |  6 +++---
 misc/dumpe2fs.c      | 14 ++++++++++----
 misc/util.c          |  7 +++++--
 tests/filter.sed     |  1 +
 6 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c
index 9b70145..7cfc269 100644
--- a/debugfs/debugfs.c
+++ b/debugfs/debugfs.c
@@ -2446,8 +2446,10 @@ void do_dump_mmp(int argc EXT2FS_ATTR((unused)), char *argv[],
 	fprintf(stdout, "check_interval: %d\n", mmp_s->mmp_check_interval);
 	fprintf(stdout, "sequence: %08x\n", mmp_s->mmp_seq);
 	fprintf(stdout, "time: %lld -- %s", mmp_s->mmp_time, ctime(&t));
-	fprintf(stdout, "node_name: %s\n", mmp_s->mmp_nodename);
-	fprintf(stdout, "device_name: %s\n", mmp_s->mmp_bdevname);
+	fprintf(stdout, "node_name: %.*s\n",
+		(int)sizeof(mmp_s->mmp_nodename), (char *)mmp_s->mmp_nodename);
+	fprintf(stdout, "device_name: %.*s\n",
+		(int)sizeof(mmp_s->mmp_bdevname), (char *)mmp_s->mmp_bdevname);
 	fprintf(stdout, "magic: 0x%x\n", mmp_s->mmp_magic);
 	fprintf(stdout, "checksum: 0x%08x\n", mmp_s->mmp_checksum);
 }
diff --git a/e2fsck/util.c b/e2fsck/util.c
index db6a1cc..07885ab 100644
--- a/e2fsck/util.c
+++ b/e2fsck/util.c
@@ -777,8 +777,12 @@ void dump_mmp_msg(struct mmp_struct *mmp, const char *fmt, ...)
 		printf("    mmp_sequence: %08x\n", mmp->mmp_seq);
 		printf("    mmp_update_date: %s", ctime(&t));
 		printf("    mmp_update_time: %lld\n", mmp->mmp_time);
-		printf("    mmp_node_name: %s\n", mmp->mmp_nodename);
-		printf("    mmp_device_name: %s\n", mmp->mmp_bdevname);
+		printf("    mmp_node_name: %.*s\n",
+		       (int)sizeof(mmp->mmp_nodename),
+		       (char *)mmp->mmp_nodename);
+		printf("    mmp_device_name: %.*s\n",
+		       (int)sizeof(mmp->mmp_bdevname),
+		       (char *)mmp->mmp_bdevname);
 	}
 }
 
diff --git a/lib/ext2fs/ext2_fs.h b/lib/ext2fs/ext2_fs.h
index 3165b38..79816b6 100644
--- a/lib/ext2fs/ext2_fs.h
+++ b/lib/ext2fs/ext2_fs.h
@@ -1098,9 +1098,9 @@ struct ext2_dir_entry_tail {
 struct mmp_struct {
 	__u32	mmp_magic;		/* Magic number for MMP */
 	__u32	mmp_seq;		/* Sequence no. updated periodically */
-	__u64	mmp_time;		/* Time last updated */
-	char	mmp_nodename[64];	/* Node which last updated MMP block */
-	char	mmp_bdevname[32];	/* Bdev which last updated MMP block */
+	__u64	mmp_time;		/* Time last updated (seconds) */
+	__u8	mmp_nodename[64];	/* Node updating MMP block, no NUL? */
+	__u8	mmp_bdevname[32];	/* Bdev updating MMP block, no NUL? */
 	__u16	mmp_check_interval;	/* Changed mmp_check_interval */
 	__u16	mmp_pad1;
 	__u32	mmp_pad2[226];
diff --git a/misc/dumpe2fs.c b/misc/dumpe2fs.c
index 9a6f586..a6053d2 100644
--- a/misc/dumpe2fs.c
+++ b/misc/dumpe2fs.c
@@ -439,8 +439,12 @@ static int check_mmp(ext2_filsys fs)
 				time_t mmp_time = mmp->mmp_time;
 
 				fprintf(stderr,
-					"%s: MMP last updated by '%s' on %s",
-					program_name, mmp->mmp_nodename,
+					"%s: MMP update by '%.*s%.*s' at %s",
+					program_name,
+					(int)sizeof(mmp->mmp_nodename),
+					(char *)mmp->mmp_nodename,
+					(int)sizeof(mmp->mmp_bdevname),
+					(char *)mmp->mmp_bdevname,
 					ctime(&mmp_time));
 			}
 			retval = 1;
@@ -489,8 +493,10 @@ static void print_mmp_block(ext2_filsys fs)
 	printf("    mmp_sequence: %#08x\n", mmp->mmp_seq);
 	printf("    mmp_update_date: %s", ctime(&mmp_time));
 	printf("    mmp_update_time: %lld\n", mmp->mmp_time);
-	printf("    mmp_node_name: %s\n", mmp->mmp_nodename);
-	printf("    mmp_device_name: %s\n", mmp->mmp_bdevname);
+	printf("    mmp_node_name: %.*s\n",
+	       (int)sizeof(mmp->mmp_nodename), (char *)mmp->mmp_nodename);
+	printf("    mmp_device_name: %.*s\n",
+	       (int)sizeof(mmp->mmp_bdevname), (char *)mmp->mmp_bdevname);
 }
 
 static void parse_extended_opts(const char *opts, blk64_t *superblock,
diff --git a/misc/util.c b/misc/util.c
index 7799158..6239b36 100644
--- a/misc/util.c
+++ b/misc/util.c
@@ -288,7 +288,10 @@ void dump_mmp_msg(struct mmp_struct *mmp, const char *msg)
 	if (mmp) {
 		time_t t = mmp->mmp_time;
 
-		printf("MMP error info: last update: %s node: %s device: %s\n",
-		       ctime(&t), mmp->mmp_nodename, mmp->mmp_bdevname);
+		printf("MMP error info: node: %.*s, device: %.*s, updated: %s",
+		       (int)sizeof(mmp->mmp_nodename),
+		       (char *)mmp->mmp_nodename,
+		       (int)sizeof(mmp->mmp_bdevname),
+		       (char *)mmp->mmp_bdevname, ctime(&t));
 	}
 }
diff --git a/tests/filter.sed b/tests/filter.sed
index f37986c..796186e 100644
--- a/tests/filter.sed
+++ b/tests/filter.sed
@@ -37,3 +37,4 @@ s/mmp_node_name: .*/mmp_node_name: test_node/
 s/mmp_update_date: .*/mmp_update_date: test date/
 s/mmp_update_time: .*/mmp_update_time: test_time/
 s/MMP last updated by '.*' on .*/MMP last updated by 'test_node' on test date/
+s/MMP update by '.*' at .*/MMP last updated by 'test_node' on test date/
-- 
1.8.0

Powered by blists - more mailing lists