svn commit: r362359 - head/sys/ufs/ffs

[Kirk McKusick]

Author: mckusick
Date: Fri Jun 19 01:04:25 2020
New Revision: 362359
URL: https://svnweb.freebsd.org/changeset/base/362359

Log:
  The binary representation of the superblock (the fs structure) is written
  out verbatim to the disk: see ffs_sbput() in sys/ufs/ffs/ffs_subr.c.
  It contains a pointer to the fs_summary_info structure. This pointer
  value inadvertently causes garbage to be stored. It is garbage because
  the pointer to the fs_summary_info structure is the address the then
  current stack or heap. Although a mere pointer does not reveal anything
  useful (like a part of a private key) to an attacker, garbage output
  deteriorates reproducibility.
  
  This commit zeros out the pointer to the fs_summary_info structure
  before writing the out the superblock.
  
  Reviewed by:  kib
  Tested by:    Peter Holm
  PR:           246983
  Sponsored by: Netflix

Modified:
  head/sys/ufs/ffs/ffs_subr.c
  head/sys/ufs/ffs/ffs_vfsops.c

Modified: head/sys/ufs/ffs/ffs_subr.c
==============================================================================
--- head/sys/ufs/ffs/ffs_subr.c	Fri Jun 19 01:02:53 2020	(r362358)
+++ head/sys/ufs/ffs/ffs_subr.c	Fri Jun 19 01:04:25 2020	(r362359)
@@ -50,7 +50,6 @@ uint32_t ffs_calc_sbhash(struct fs *);
 struct malloc_type;
 #define UFS_MALLOC(size, type, flags) malloc(size)
 #define UFS_FREE(ptr, type) free(ptr)
-#define UFS_TIME time(NULL)
 /*
  * Request standard superblock location in ffs_sbget
  */
@@ -78,7 +77,6 @@ struct malloc_type;
 
 #define UFS_MALLOC(size, type, flags) malloc(size, type, flags)
 #define UFS_FREE(ptr, type) free(ptr, type)
-#define UFS_TIME time_second
 
 #endif /* _KERNEL */
 
@@ -349,11 +347,24 @@ ffs_sbput(void *devfd, struct fs *fs, off_t loc,
 		}
 	}
 	fs->fs_fmod = 0;
-	fs->fs_time = UFS_TIME;
+#ifndef _KERNEL
+	{
+		struct fs_summary_info *fs_si;
+
+		fs->fs_time = time(NULL);
+		/* Clear the pointers for the duration of writing. */
+		fs_si = fs->fs_si;
+		fs->fs_si = NULL;
+		fs->fs_ckhash = ffs_calc_sbhash(fs);
+		error = (*writefunc)(devfd, loc, fs, fs->fs_sbsize);
+		fs->fs_si = fs_si;
+	}
+#else /* _KERNEL */
+	fs->fs_time = time_second;
 	fs->fs_ckhash = ffs_calc_sbhash(fs);
-	if ((error = (*writefunc)(devfd, loc, fs, fs->fs_sbsize)) != 0)
-		return (error);
-	return (0);
+	error = (*writefunc)(devfd, loc, fs, fs->fs_sbsize);
+#endif /* _KERNEL */
+	return (error);
 }
 
 /*

Modified: head/sys/ufs/ffs/ffs_vfsops.c
==============================================================================
--- head/sys/ufs/ffs/ffs_vfsops.c	Fri Jun 19 01:02:53 2020	(r362358)
+++ head/sys/ufs/ffs/ffs_vfsops.c	Fri Jun 19 01:04:25 2020	(r362359)
@@ -2293,6 +2293,7 @@ ffs_use_bwrite(void *devfd, off_t loc, void *buf, int 
 	bcopy((caddr_t)fs, bp->b_data, (u_int)fs->fs_sbsize);
 	fs = (struct fs *)bp->b_data;
 	ffs_oldfscompat_write(fs, ump);
+	fs->fs_si = NULL;
 	/* Recalculate the superblock hash */
 	fs->fs_ckhash = ffs_calc_sbhash(fs);
 	if (devfdp->suspended)