Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 10 Jul 2012 17:32:52 +0000
From:      gpf@FreeBSD.org
To:        svn-soc-all@FreeBSD.org
Subject:   socsvn commit: r239244 - in soc2012/gpf/pefs_kmod: sbin/pefs sys/fs/pefs
Message-ID:  <20120710173252.A1C861065670@hub.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: gpf
Date: Tue Jul 10 17:32:52 2012
New Revision: 239244
URL: http://svnweb.FreeBSD.org/socsvn/?view=rev&rev=239244

Log:
  take care of dynamic memory
  
  - free RB tree kept for hardlinks
  - new allocation function for struct file_header. moving all initialization
  code to this function really helps.
  - new function to free struct file_header
  - free global file header tailq
  - also fixing a few leaks in case of error here and there.
  

Modified:
  soc2012/gpf/pefs_kmod/sbin/pefs/pefs_checksum.c
  soc2012/gpf/pefs_kmod/sbin/pefs/pefs_ctl.c
  soc2012/gpf/pefs_kmod/sys/fs/pefs/pefs_checksum.c

Modified: soc2012/gpf/pefs_kmod/sbin/pefs/pefs_checksum.c
==============================================================================
--- soc2012/gpf/pefs_kmod/sbin/pefs/pefs_checksum.c	Tue Jul 10 15:02:29 2012	(r239243)
+++ soc2012/gpf/pefs_kmod/sbin/pefs/pefs_checksum.c	Tue Jul 10 17:32:52 2012	(r239244)
@@ -57,7 +57,7 @@
 
 #include "pefs_ctl.h"
 
-//#define PEFS_INTEGRITY_DEBUG
+#define PEFS_INTEGRITY_DEBUG
 #if defined (PEFS_INTEGRITY_DEBUG)
 #define dprintf(a)		printf a
 #else
@@ -199,9 +199,6 @@
 	size_t buf_len;
 	struct checksum *csp;
 
-	TAILQ_INIT(&(fhp->checksums));
-	fhp->nhashes = 0;
-
 	if ((flags & PEFS_UNMOUNTED) != 0 || (flags & PEFS_NOKEY) != 0) {
 		buf = fhp->target_path;
 		buf_len = strnlen(fhp->target_path, MAXPATHLEN + 1);
@@ -272,8 +269,6 @@
 	if (fhp->target_path != NULL)
 		return (pefs_compute_symlink_checksum(fhp, md, hash_len, flags));
 
-	TAILQ_INIT(&(fhp->checksums));
-
 	/* XXXgpf: what happens if file size > 2^64? */
 	if (fstat(fhp->fd, &sb) != 0) {
 		warn("cannot stat file %s", fhp->path);
@@ -295,7 +290,6 @@
 		}
 	}
 
-	fhp->nhashes = 0;
 	offset = 0;
 	xsct.pxsct_offset = 0;
 	while (resid > 0) {
@@ -339,12 +333,14 @@
 		csp = malloc(sizeof(struct checksum));
 		if (csp == NULL) {
 			pefs_warn("memory allocation error");
+			EVP_MD_CTX_cleanup(&mdctx);
 			return (PEFS_ERR_SYS);
 		}
 		csp->hash = malloc(hash_len);
 		if (csp->hash == NULL) {
 			pefs_warn("memory allocation error");
 			free(csp);
+			EVP_MD_CTX_cleanup(&mdctx);
 			return (PEFS_ERR_SYS);
 		}
 
@@ -436,29 +432,62 @@
 	return (0);
 }
 
+static struct file_header *
+pefs_allocate_file_header(void)
+{
+	struct file_header *fhp;
+
+	fhp = malloc(sizeof(struct file_header));
+	if (fhp == NULL) {
+		pefs_warn("memory allocation error");
+		return (NULL);
+	}
+
+	fhp->nhashes = 0;
+	fhp->offset_to_checksums = 0;
+	fhp->file_id = 0;
+	fhp->fd = -1;
+	fhp->pfd = -1;
+	fhp->found = 0;
+
+	fhp->target_path = NULL;
+
+	fhp->path[0] = '\0';
+	fhp->dirpath[0] = '\0';
+	fhp->filename[0] = '\0';
+
+	TAILQ_INIT(&(fhp->checksums));
+
+	return (fhp);
+}
+
+static void
+pefs_free_file_header(struct file_header *fhp)
+{
+	struct checksum *csp, *tcsp;
+	if (fhp != NULL) {
+		TAILQ_FOREACH_SAFE(csp, &(fhp->checksums), checksum_entries, tcsp) {
+			TAILQ_REMOVE(&(fhp->checksums), csp, checksum_entries);
+			if (csp->hash != NULL)
+				free(csp->hash);
+			free(csp);
+		}
+		if (fhp->target_path != NULL)
+			free(fhp->target_path);
+		free(fhp);
+	}
+}
+
 static void
 pefs_free_hash_table(struct cuckoo_hash_table *chtp)
 {
 	struct bucket *bp;
-	struct file_header *fhp;
-	struct checksum *csp, *tcsp;
 	uint32_t i;
 
 	if (chtp->buckets1 != NULL) {
 		for (i = 0; i < chtp->size; i++) {
 			bp = &chtp->buckets1[i];
-			fhp = bp->fhp;
-			if (fhp != NULL) {
-				TAILQ_FOREACH_SAFE(csp, &(fhp->checksums), checksum_entries, tcsp) {
-					TAILQ_REMOVE(&(fhp->checksums), csp, checksum_entries);
-					if (csp->hash != NULL)
-						free(csp->hash);
-					free(csp);
-				}
-				if (fhp->target_path != NULL)
-					free(fhp->target_path);
-				free(fhp);
-			}
+			pefs_free_file_header(bp->fhp);
 		}
 		free(chtp->buckets1);
 	}
@@ -466,23 +495,23 @@
 	if (chtp->buckets2 != NULL) {
 		for (i = 0; i < chtp->size; i++) {
 			bp = &chtp->buckets2[i];
-			fhp = bp->fhp;
-			if (fhp != NULL) {
-				TAILQ_FOREACH_SAFE(csp, &(fhp->checksums), checksum_entries, tcsp) {
-					TAILQ_REMOVE(&(fhp->checksums), csp, checksum_entries);
-					if (csp->hash != NULL)
-						free(csp->hash);
-					free(csp);
-				}
-				if (fhp->target_path != NULL)
-					free(fhp->target_path);
-				free(fhp);
-			}
+			pefs_free_file_header(bp->fhp);
 		}
 		free(chtp->buckets2);
 	}
 }
 
+static void
+pefs_free_file_header_tail(struct file_header_head *fh_headp)
+{
+	struct file_header *fhp, *tfhp;
+
+	TAILQ_FOREACH_SAFE(fhp, fh_headp, file_header_entries, tfhp) {
+		TAILQ_REMOVE(fh_headp, fhp, file_header_entries);
+		pefs_free_file_header(fhp);
+	}
+}
+
 static uint32_t
 pefs_hash1(struct cuckoo_hash_table *chtp, struct file_header *fhp)
 {
@@ -830,6 +859,18 @@
 		return 0;
 }
 
+static void
+pefs_rb_free(struct hardlink_head *hlc_headp)
+{
+	struct hardlink_counter *cur, *next;
+
+	for (cur = RB_MIN(hardlink_head, hlc_headp); cur != NULL; cur = next) {
+		next = RB_NEXT(hardlink_head, hlc_headp, cur);
+		RB_REMOVE(hardlink_head, hlc_headp, cur);
+		free(cur);
+	}
+}
+
 /* open a file and perform various semantic checks on it */
 static int
 pefs_open_semantic_checks(struct file_header *fhp, struct statfs *fsp, struct hardlink_head *hlc_headp, int flags)
@@ -842,10 +883,6 @@
 	size_t target_path_size;
 	int nchars;
 
-	/* initialize file descriptors in case error occurs */
-	fhp->fd = -1;
-	fhp->pfd = -1;
-
 	/* retrieve dirpath & filename */
 	strlcpy(dirbuf, fhp->path, sizeof(dirbuf));
 	strlcpy(namebuf, fhp->path, sizeof(namebuf));
@@ -873,8 +910,6 @@
 	}
 
 	if (S_ISLNK(sb.st_mode) != 0) {
-		fhp->target_path = NULL;
-
 		fhp->pfd = open(fhp->dirpath, O_RDONLY);
 		if (fhp->pfd < 0) {
 			warn("cannot open %s", fhp->dirpath);
@@ -931,8 +966,6 @@
 		}
 		return (0);
 	}
-	else
-		fhp->target_path = NULL;
 
 	fhp->fd = open(fhp->path, O_RDONLY | O_NOFOLLOW);
 	if (fhp->fd < 0) {
@@ -997,9 +1030,8 @@
 		buf[strnlen(buf, sizeof(buf)) - 1] = '\0';
 	dprintf(("\nnext file entry=%s!\n", buf));
 
-	fhp = malloc(sizeof(struct file_header));
+	fhp = pefs_allocate_file_header();
 	if (fhp == NULL) {
-		warn("memory allocation error");
 		*error = PEFS_ERR_SYS;
 		return (NULL);
 	}
@@ -1058,18 +1090,21 @@
 		error = pefs_open_semantic_checks(fhp, &fs, &hlc_head, 0);
 		if (error != 0) {
 			pefs_close_file(fhp);
+			pefs_free_file_header(fhp);
 			return (error);
 		}
 
 		error = pefs_get_file_id(fhp, 0);
 		if (error != 0) {
 			pefs_close_file(fhp);
+			pefs_free_file_header(fhp);
 			return (error);
 		}
 
 		error = pefs_compute_file_checksums(fhp, md, hash_len, 0);
 		if (error != 0) {
 			pefs_close_file(fhp);
+			pefs_free_file_header(fhp);
 			return (error);
 		}
 
@@ -1083,7 +1118,7 @@
 
 	pefs_rb_print(&hlc_head);
 	pefs_rb_warn(&hlc_head);
-	/* XXXgpf: [TODO] rb_free */
+	pefs_rb_free(&hlc_head);
 
 	error = pefs_allocate_hash_table(chtp, nfiles, PEFS_EXTEND);
 	if (error != 0)
@@ -1511,18 +1546,16 @@
 	int error;
 
 	//dprintf(("bucket offset = %d\n", *buckets_offset));
-	fhp = malloc(sizeof(struct file_header));
-	if (fhp == NULL) {
-		pefs_warn("memory allocation error");
+	fhp = pefs_allocate_file_header();
+	if (fhp == NULL)
 		return (PEFS_ERR_SYS);
-	}
 
 	error = pefs_read_file_header(fdin, fhp, buckets_offset);
 	if (error != 0)
 		return (error);
 
 	if (fhp->nhashes == 0) {
-		free(fhp);
+		pefs_free_file_header(fhp);
 		fhp = NULL;
 	}
 	bp->fhp = fhp;
@@ -1556,9 +1589,7 @@
 	TAILQ_INSERT_TAIL(&(fhp->checksums), csp, checksum_entries);
 }
 
-/*
- * XXXgpf: [TODO] comments
- */
+/* Create in memory checksum database by reading .pefs.checksum file. */
 static int
 pefs_read_checksum_file(int fdin, struct checksum_file_header *cfhp, struct cuckoo_hash_table *chtp)
 {
@@ -1579,10 +1610,7 @@
 
 		fhp = bp->fhp;
 		if (fhp != NULL) {
-			TAILQ_INIT(&(fhp->checksums));
 			hashes_offset = fhp->offset_to_checksums;
-			fhp->found = 0;
-			fhp->target_path = NULL;
 
 			for (k = 0; k < fhp->nhashes; k++) {
 				csp = malloc(sizeof(struct checksum));
@@ -1613,10 +1641,7 @@
 
 		fhp = bp->fhp;
 		if (fhp != NULL) {
-			TAILQ_INIT(&(fhp->checksums));
 			hashes_offset = fhp->offset_to_checksums;
-			fhp->found = 0;
-			fhp->target_path = NULL;
 
 			for (k = 0; k < fhp->nhashes; k++) {
 				csp = malloc(sizeof(struct checksum));
@@ -1728,9 +1753,8 @@
 			/* FALLTHROUGH */
 			case DT_REG:
 			case DT_LNK:
-				fhp = malloc(sizeof(struct file_header));
+				fhp = pefs_allocate_file_header();
 				if (fhp == NULL) {
-					warn("memory allocation error");
 					closedir(dirp);
 					return (PEFS_ERR_SYS);
 				}
@@ -1739,7 +1763,7 @@
 				error = pefs_open_semantic_checks(fhp, fsp, NULL, flags);
 				if (error != 0) {
 					closedir(dirp);
-					free(fhp);
+					pefs_free_file_header(fhp);
 					return (error);
 				}
 
@@ -1747,14 +1771,14 @@
 				if (error != 0) {
 					closedir(dirp);
 					pefs_close_file(fhp);
-					free(fhp);
+					pefs_free_file_header(fhp);
 					return (error);
 				}
 
 				indexfhp = pefs_cuckoo_lookup(chtp, fhp);
 				if (indexfhp == NULL) {
 					pefs_close_file(fhp);
-					free(fhp);
+					pefs_free_file_header(fhp);
 					break;
 				}
 				indexfhp->found = 1;
@@ -1763,7 +1787,7 @@
 				if (error != 0) {
 					closedir(dirp);
 					pefs_close_file(fhp);
-					free(fhp);
+					pefs_free_file_header(fhp);
 					return (error);
 				}
 
@@ -1773,7 +1797,7 @@
 					warn("cannot stat file %s", fhp->path);
 					closedir(dirp);
 					pefs_close_file(fhp);
-					free(fhp);
+					pefs_free_file_header(fhp);
 					return (PEFS_ERR_SYS);
 				}
 
@@ -1781,7 +1805,7 @@
 				if (error != 0) {
 					closedir(dirp);
 					pefs_close_file(fhp);
-					free(fhp);
+					pefs_free_file_header(fhp);
 					return (error);
 				}
 
@@ -1916,8 +1940,8 @@
 
 out:
 	pefs_free_hash_table(&cht);
-	/* XXXgpf: [TODO] rb_free */
-	/* XXXgpf: [TODO] fh_free */
+	pefs_rb_free(&hlc_head);
+	pefs_free_file_header_tail(&fh_head);
 	return (error);
 }
 

Modified: soc2012/gpf/pefs_kmod/sbin/pefs/pefs_ctl.c
==============================================================================
--- soc2012/gpf/pefs_kmod/sbin/pefs/pefs_ctl.c	Tue Jul 10 15:02:29 2012	(r239243)
+++ soc2012/gpf/pefs_kmod/sbin/pefs/pefs_ctl.c	Tue Jul 10 17:32:52 2012	(r239244)
@@ -1020,8 +1020,11 @@
  * .pefs.checksum is created under $PWD. path should be a directory,
  * outside of target pefs filesystem.
  *
- * When $command is run, filesystem must be mounted with pefs, and 
- * user must have supplied the key.
+ * When $command is run, filesystem must be mounted with pefs, and
+ * user must have supplied the necessary key(s).
+ *
+ * [TODO] a flag that allows $command to turn on immutable flags for
+ * every file that requires integrity checking.
  */
 static int
 pefs_addchecksum(int argc, char *argv[])
@@ -1120,8 +1123,7 @@
  * By default, pefs will assume that filesystem is mounted and user
  * has provided key.
  *
- * Verifying the integrity of the checksum file itself via a signature
- * remains a major TODO.
+ * [TODO] Verify the integrity of the checksum file itself via a signature.
  */
 static int
 pefs_verify(int argc, char *argv[])
@@ -1172,6 +1174,10 @@
 	if ((flags & PEFS_UNMOUNTED) == 0)
 		initfsroot(argc, argv, 0, fsroot, sizeof(fsroot));
 	else {
+		/*
+		 * XXXgpf: should i also check that fsroot path belongs to
+		 * a non pefs filesystem?
+		 */
 		strlcpy(fsroot, argv[0], sizeof(fsroot));
 		if (stat(fsroot, &sb) != 0) {
 			warn("cannot stat fs root: %s", fsroot);
@@ -1196,6 +1202,8 @@
 	return (error);
 }
 
+/* XXXgpf: [TODO] a command that returns the file id of a file (name MAC) */
+
 static void
 pefs_usage_alg(void)
 {

Modified: soc2012/gpf/pefs_kmod/sys/fs/pefs/pefs_checksum.c
==============================================================================
--- soc2012/gpf/pefs_kmod/sys/fs/pefs/pefs_checksum.c	Tue Jul 10 15:02:29 2012	(r239243)
+++ soc2012/gpf/pefs_kmod/sys/fs/pefs/pefs_checksum.c	Tue Jul 10 17:32:52 2012	(r239244)
@@ -79,6 +79,10 @@
 	start = &(pcs->pcs_table1[pos * PEFS_HT_CELL_SIZE]);
 	p = start;
 
+	/* 
+	 * XXXgpf: [TODO] move the reading of a checksum index entry to a different function.
+	 * Also, perhaps a macro to tell if an index entry is empty or not (nhashes == 0).
+	 */
 	memcpy(&(target_pcie.pcie_nhashes), p, sizeof(target_pcie.pcie_nhashes));
 	target_pcie.pcie_nhashes = le32toh(target_pcie.pcie_nhashes);
 	if (target_pcie.pcie_nhashes != 0) {



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120710173252.A1C861065670>