Re: veriexec

[Paul Goyette <paul%whooppee.com@localhost>]

On Tue, 29 Aug 2017, Paul Goyette wrote:

> While looking at Sevan's recent PR, I notice a couple of problems with the 
> current code.
>
> In sys/kern_veriexec.c routine veriexec_file_add(), at line 1072 we allocate 
> a vfe entry, and initialize the rw_lock contained therein.
>
> Then there are some error branches at lines 1082 and 1090 that simply "goto 
> out" without ever destroying the rw_lock nor free()ing the vfe.
>
> (diffs removed, since they were wrong!)
>
> Comments or other suggestions?

On IRC, riastradh@ pointed out that the above diffs are not quite right, 
since in the success case we need to avoid destroying the rwlock and 
free()ing the vfe entry.

Revised diffs:

Index: kern_veriexec.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_veriexec.c,v
retrieving revision 1.12
diff -u -p -r1.12 kern_veriexec.c
--- kern_veriexec.c	12 Apr 2017 10:30:02 -0000	1.12
+++ kern_veriexec.c	29 Aug 2017 02:34:57 -0000
@@ -1079,7 +1079,7 @@ veriexec_file_add(struct lwp *l, prop_di
 		log(LOG_ERR, "Veriexec: Invalid or unknown fingerprint type "
 		    "`%s' for file `%s'.\n", fp_type, file);
 		error = EOPNOTSUPP;
-		goto out;
+		goto free_out;
 	}

 	if (prop_data_size(prop_dictionary_get(dict, "fp")) !=
@@ -1087,7 +1087,7 @@ veriexec_file_add(struct lwp *l, prop_di
 		log(LOG_ERR, "Veriexec: Bad fingerprint length for `%s'.\n",
 		    file);
 		error = EINVAL;
-		goto out;
+		goto free_out;
 	}

 	vfe->fp = kmem_alloc(vfe->ops->hash_len, KM_SLEEP);
@@ -1158,6 +1158,12 @@ veriexec_file_add(struct lwp *l, prop_di
   unlock_out:
 	rw_exit(&veriexec_op_lock);

+  free_out:
+	if (error) {
+		rw_destroy(&vfe->lock);
+		kmem_free(vfe, sizeof(*vfe));
+	}
+
   out:
 	vrele(vp);
 	if (error)


+------------------+--------------------------+----------------------------+
| Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:          |
| (Retired)        | FA29 0E3B 35AF E8AE 6651 | paul at whooppee dot com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd dot org |
+------------------+--------------------------+----------------------------+