Correct fsck journal-recovery code to update a cylinder-group

check-hash after making changes to the cylinder group. The problem
was that the journal-recovery code was calling the libufs bwrite()
function instead of the cgput() function. The cgput() function updates
the cylinder-group check-hash before writing the cylinder group.

This change required the additions of the cgget() and cgput() functions
to the libufs API to avoid a gratuitous bcopy of every cylinder group
to be read or written. These new functions have been added to the
libufs manual pages. This was the first opportunity that I have had
to use and document the use of the EDOOFUS error code.

Reviewed by: kib
Reported by: emaste and others

lib/libufs/Makefile

@@ -12,7 +12,10 @@ MAN=	bread.3 cgread.3 libufs.3 sbread.3 ufs_disk_close.3
 MLINKS+= bread.3 bwrite.3
 MLINKS+= bread.3 berase.3
 MLINKS+= cgread.3 cgread1.3
+MLINKS+= cgread.3 cgget.3
+MLINKS+= cgread.3 cgwrite.3
 MLINKS+= cgread.3 cgwrite1.3
+MLINKS+= cgread.3 cgput.3
 MLINKS+= sbread.3 sbwrite.3
 MLINKS+= ufs_disk_close.3 ufs_disk_fillout.3
 MLINKS+= ufs_disk_close.3 ufs_disk_fillout_blank.3

lib/libufs/cgread.3

@@ -2,19 +2,22 @@
 .\" Date:	June 04, 2003
 .\" Description:
 .\" 	Manual page for libufs functions:
+.\"		cgget(3)
+.\"		cgput(3)
 .\"		cgread(3)
 .\"		cgread1(3)
+.\"		cgwrite(3)
 .\"		cgwrite1(3)
 .\"
 .\" This file is in the public domain.
 .\"
 .\" $FreeBSD$
 .\"
-.Dd June 4, 2003
+.Dd January 19, 2018
 .Dt CGREAD 3
 .Os
 .Sh NAME
-.Nm cgread , cgread1, cgwrite1
+.Nm cgget , cgput , cgread , cgread1 , cgwrite , cgwrite1
 .Nd read/write cylinder groups of UFS disks
 .Sh LIBRARY
 .Lb libufs
@@ -26,30 +29,72 @@
 .In ufs/ffs/fs.h
 .In libufs.h
 .Ft int
+.Fn cgget "struct uufsd *disk" "int cg" "struct cg *cgp"
+.Ft int
+.Fn cgput "struct uufsd *disk" "struct cg *cgp"
+.Ft int
 .Fn cgread "struct uufsd *disk"
 .Ft int
-.Fn cgread1 "struct uufsd *disk" "int c"
+.Fn cgread1 "struct uufsd *disk" "int cg"
 .Ft int
-.Fn cgwrite1 "struct uufsd *disk" "int c"
+.Fn cgwrite "struct uufsd *disk"
+.Ft int
+.Fn cgwrite1 "struct uufsd *disk" "int cg"
 .Sh DESCRIPTION
 The
-.Fn cgread
+.Fn cgget ,
+.Fn cgread ,
 and
 .Fn cgread1
 functions provide cylinder group reads for
 .Xr libufs 3
 consumers.
 The
+.Fn cgput ,
+.Fn cgwrite ,
+and
+.Fn cgwrite1
+functions provide cylinder group writes for
+.Xr libufs 3
+consumers.
+.Pp
+The
+.Fn cgget
+function reads the cylinder group specified by
+.Fa cg
+into the buffer pointed to by
+.Fa cgp
+from the disk referenced by the user-land UFS-disk structure.
+The
+.Fn cgget
+function is the only cylinder group read function that is safe to use
+in threaded applications.
+.Pp
+The
+.Fn cgput
+function writes the cylinder group specified by
+.Va cgp
+to the disk referenced by the user-land UFS-disk structure.
+The
+.Fn cgput
+function is the only cylinder group write function that is safe to use
+in threaded applications.
+Note that the
+.Fn cgput
+function needs to be called only if the cylinder group has been
+modified and the on-disk copy needs to be updated.
+.Pp
+The
 .Fn cgread1
-function reads from one cylinder group, specified by
-.Fa c
+function reads from the cylinder group specified by
+.Fa cg
 into the
 .Va d_cg
-field of a userland UFS disk structure.
+cylinder-group structure in a user-land UFS-disk structure.
 It sets the
 .Va d_lcg
 field to the cylinder group number
-.Fa c .
+.Fa cg .
 .Pp
 The
 .Fn cgread
@@ -65,34 +110,57 @@ field, and then incrementing the
 field.
 .Pp
 The
-.Fn cgwrite1
-function stores cylinder group specified by
-.Fa c
-from
+.Fn cgwrite
+function stores on disk the cylinder group held in the
 .Va d_cg
-field of a userland UFS disk structure on disk.
+cylinder-group structure in a user-land UFS-disk structure.
+.Pp
+The
+.Fn cgwrite1
+function provides no additional functionality over the
+.Fn cgwrite
+function as there is only one place that a given cylinder group
+can correctly be written.
+If the caller gets the
+.Fa cg
+parameter wrong, the function fails with the error
+.Er EDOOFUS .
+This function remains only to provide backward compatibility.
 .Sh RETURN VALUES
-Both functions return 0 if there are no more cylinder groups to read,
-1 if there are more cylinder groups, and \-1 on error.
-.Sh ERRORS
-The function
+The
 .Fn cgread
-may fail and set
+function returns 0 if there are no more cylinder groups to read,
+1 if there are more cylinder groups, and \-1 on error.
+The
+.Fn cgread1
+function returns 1 on success and \-1 on error.
+The other functions return 0 on success and \-1 on error;
+.Sh ERRORS
+The
+.Fn cgget ,
+.Fn cgread ,
+and
+.Fn cgread1
+functions may fail and set
 .Va errno
 for any of the errors specified for the library function
 .Xr bread 3 .
 .Pp
-The function
-.Fn cgread1
-has semantically identical failure conditions to those of
-.Fn cgread .
-.Pp
-The function
+The
+.Fn cgput ,
+.Fn cgwrite ,
+and
 .Fn cgwrite1
-may fail and set
+functions may fail and set
 .Va errno
 for any of the errors specified for the library function
 .Xr bwrite 3 .
+Additionally the
+.Fn cgwrite1
+will return the
+.Er EDOOFUS
+error if the cylinder group specified does not match the
+cylinder group that it is requesting to write.
 .Sh SEE ALSO
 .Xr bread 3 ,
 .Xr bwrite 3 ,

lib/libufs/cgroup.c

@@ -182,47 +182,85 @@ cgialloc(struct uufsd *disk)
 int
 cgread(struct uufsd *disk)
 {
+
+	if (disk->d_ccg >= disk->d_fs.fs_ncg)
+		return (0);
 	return (cgread1(disk, disk->d_ccg++));
 }
 
 int
 cgread1(struct uufsd *disk, int c)
+{
+
+	if ((cgget(disk, c, &disk->d_cg)) == 0)
+		return (1);
+	return (-1);
+}
+
+int
+cgget(struct uufsd *disk, int cg, struct cg *cgp)
 {
 	struct fs *fs;
+	uint32_t cghash, calchash;
 
 	fs = &disk->d_fs;
-
-	if ((unsigned)c >= fs->fs_ncg) {
-		return (0);
-	}
-	if (bread(disk, fsbtodb(fs, cgtod(fs, c)), disk->d_cgunion.d_buf,
-	    fs->fs_bsize) == -1) {
+	if (bread(disk, fsbtodb(fs, cgtod(fs, cg)), (void *)cgp,
+	    fs->fs_cgsize) == -1) {
 		ERROR(disk, "unable to read cylinder group");
 		return (-1);
 	}
-	disk->d_lcg = c;
-	return (1);
+	calchash = cgp->cg_ckhash;
+	if ((fs->fs_metackhash & CK_CYLGRP) != 0) {
+		cghash = cgp->cg_ckhash;
+		cgp->cg_ckhash = 0;
+		calchash = calculate_crc32c(~0L, (void *)cgp, fs->fs_cgsize);
+		cgp->cg_ckhash = cghash;
+	}
+	if (cgp->cg_ckhash != calchash || !cg_chkmagic(cgp) ||
+	    cgp->cg_cgx != cg) {
+		ERROR(disk, "cylinder group checks failed");
+		errno = EIO;
+		return (-1);
+	}
+	disk->d_lcg = cg;
+	return (0);
 }
 
 int
 cgwrite(struct uufsd *disk)
 {
-	return (cgwrite1(disk, disk->d_lcg));
+
+	return (cgput(disk, &disk->d_cg));
 }
 
 int
-cgwrite1(struct uufsd *disk, int c)
+cgwrite1(struct uufsd *disk, int cg)
+{
+	static char errmsg[BUFSIZ];
+
+	if (cg == disk->d_cg.cg_cgx)
+		return (cgput(disk, &disk->d_cg));
+	snprintf(errmsg, BUFSIZ, "Cylinder group %d in buffer does not match "
+	    "the cylinder group %d that cgwrite1 requested",
+	    disk->d_cg.cg_cgx, cg);
+	ERROR(disk, errmsg);
+	errno = EDOOFUS;
+	return (-1);
+}
+
+int
+cgput(struct uufsd *disk, struct cg *cgp)
 {
 	struct fs *fs;
 
 	fs = &disk->d_fs;
 	if ((fs->fs_metackhash & CK_CYLGRP) != 0) {
-		disk->d_cg.cg_ckhash = 0;
-		disk->d_cg.cg_ckhash =
-		    calculate_crc32c(~0L, (void *)&disk->d_cg, fs->fs_cgsize);
+		cgp->cg_ckhash = 0;
+		cgp->cg_ckhash =
+		    calculate_crc32c(~0L, (void *)cgp, fs->fs_cgsize);
 	}
-	if (bwrite(disk, fsbtodb(fs, cgtod(fs, c)),
-	    disk->d_cgunion.d_buf, fs->fs_bsize) == -1) {
+	if (bwrite(disk, fsbtodb(fs, cgtod(fs, cgp->cg_cgx)), cgp,
+	    fs->fs_cgsize) == -1) {
 		ERROR(disk, "unable to write cylinder group");
 		return (-1);
 	}

lib/libufs/libufs.3

@@ -7,7 +7,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd June 4, 2003
+.Dd January 19, 2018
 .Dt LIBUFS 3
 .Os
 .Sh NAME
@@ -55,8 +55,11 @@ to a string describing the error.
 .Sh SEE ALSO
 .Xr bread 3 ,
 .Xr bwrite 3 ,
+.Xr cgget 3 ,
+.Xr cgput 3 ,
 .Xr cgread 3 ,
 .Xr cgread1 3 ,
+.Xr cgwrite 3 ,
 .Xr cgwrite1 3 ,
 .Xr sbread 3 ,
 .Xr sbwrite 3 ,

lib/libufs/libufs.h

@@ -111,6 +111,8 @@ int berase(struct uufsd *, ufs2_daddr_t, ufs2_daddr_t);
 ufs2_daddr_t cgballoc(struct uufsd *);
 int cgbfree(struct uufsd *, ufs2_daddr_t, long);
 ino_t cgialloc(struct uufsd *);
+int cgget(struct uufsd *, int, struct cg *);
+int cgput(struct uufsd *, struct cg *);
 int cgread(struct uufsd *);
 int cgread1(struct uufsd *, int);
 int cgwrite(struct uufsd *);

sbin/fsck_ffs/gjournal.c

@@ -134,9 +134,8 @@ getcg(int cg)
 		if (cgc == NULL)
 			err(1, "malloc(%zu)", sizeof(*cgc));
 	}
-	if (cgread1(disk, cg) == -1)
-		err(1, "cgread1(%d)", cg);
-	bcopy(&disk->d_cg, &cgc->cgc_cg, sizeof(cgc->cgc_union));
+	if (cgget(disk, cg, &cgc->cgc_cg) == -1)
+		err(1, "cgget(%d)", cg);
 	cgc->cgc_busy = 0;
 	cgc->cgc_dirty = 0;
 	LIST_INSERT_HEAD(&cglist, cgc, cgc_next);
@@ -191,10 +190,8 @@ putcgs(void)
 		LIST_REMOVE(cgc, cgc_next);
 		ncgs--;
 		if (cgc->cgc_dirty) {
-			bcopy(&cgc->cgc_cg, &disk->d_cg,
-			    sizeof(cgc->cgc_union));
-			if (cgwrite1(disk, cgc->cgc_cg.cg_cgx) == -1)
-				err(1, "cgwrite1(%d)", cgc->cgc_cg.cg_cgx);
+			if (cgput(disk, &cgc->cgc_cg) == -1)
+				err(1, "cgput(%d)", cgc->cgc_cg.cg_cgx);
 			//printf("%s: Wrote cg=%d\n", __func__,
 			//    cgc->cgc_cg.cg_cgx);
 		}

sbin/fsck_ffs/suj.c

@@ -1892,8 +1892,7 @@ cg_write(struct suj_cg *sc)
 	 * before writing the block.
 	 */
 	fs->fs_cs(fs, sc->sc_cgx) = cgp->cg_cs;
-	if (bwrite(disk, fsbtodb(fs, cgtod(fs, sc->sc_cgx)), sc->sc_cgbuf,
-	    fs->fs_bsize) == -1)
+	if (cgput(disk, cgp) == -1)
 		err_suj("Unable to write cylinder group %d\n", sc->sc_cgx);
 }