Re: veriexec(4) maintenance

[Brett Lymn <blymn%internode.on.net@localhost>]

Jan,

I apologise for taking so long to get back to you.  I do appreciate you
taking the time to help with this.

> 
> first it is difficult to read a diff that also introduces a "notyet" operation
> on file pages.
> 

OK, I will remove those sections to make things clearer.  I still have a
dream of getting per-page signatures in to the kernel some day...

> Looks like you replace global and item rw-locks with refcounts/signals.
> I tried to understand the rationale behind and failed.  Your implementation
> seems at least vulnerable to livelocks as the writer side is missing priority.
> 

My aim was to improve multi-thread performance and remove some of the
more horrible hacks I made to make things work - like looping on the
acquisition of a rw lock.  I also was lead to believe that rw locks are
very heavy weight so I was trying to replace them with something less
onerous.

> In general, what problem(s) are you trying to solve here?
> 

The general process for veriexec is when the kernel comes up the tables
are blank.  Early in the boot process the signatures are loaded into the
kernel.  After the signatures are loaded and a file is executed the
signature for the file is evaluated and the result of the comparison
between the loaded and calculated signatures is cached which improves
the performance.

The things that I think need to be protected are:

1) If the tables are being loaded then any evaluations should wait until
the table is loaded.

2) If the signature for a file is being evaluated then any other
accesses to that entry should wait until the evaulation is done.

3) At some securelevel settings, the signatures can be updated on the
fly.  The signature entry must not be updated while it is being
accessed.

4) If the file system associated with a particular table of signatures
is being unmounted then that table must be cleared.

> What is wrong with a quite simple hashtable to hold fileid<->fingerprint mapping accessed as:
> 
> 	mutex_enter table lock
> 	lookup fileid -> item
> 	mutex_enter item
> 	mutex_exit table lock
> 
> 	while item state is busy
> 		cv_wait item cv
> 
> 	if item state is unknown
> 		set item state busy
> 		mutex_exit item
> 		compute fingerprint
> 		mutex_enter item
> 		set item state valid/invalid
> 		cv_broadcast item cv
> 
> 	Here the item state is either valid or invalid
> 
> 	mutex_exit item
> 

well, that was pretty much what my intention was but clearly I failed in
the implementation.

> 
> lock_state is sometimes VERIEXEC_UNLOCKED, sometimes IO_NODELOCKED.
> 
>   1629	veriexec_dump(struct lwp *l, prop_array_t rarray)
>   1630	{
>   1631		struct mount *mp;
>   1632		mount_iterator_t *mpiter;
>   1633	
>   1634		mountlist_iterator_init(&mpiter);
>   1635	
>   1636		while ((mp = mountlist_iterator_trynext(mpiter)) != NULL) {
>   1637			/* If it fails, the file-system is [being] unmounted. */
>   1638			if (vfs_busy(mp) != 0)
>   1639				continue;
> 
> This is completely wrong.  Operation mountlist_iterator_trynext() already
> returns the mount busy and skips mounts currently unmounting or remounting.
> Operation mountlist_iterator_next() was right or does this function get
> called from inside unmount?
> 

No, not this one, when I first did this code there was no
mountlist_iterator framework.  This dump call should ignore file systems
being unmounted.

>   1653	veriexec_flush(struct lwp *l)
> 
> This is completely wrong, see above.
> 

This one should be called on unmount.

I will fix up and simplify the locking and make sure I use the
mountlist_iterator stuff properly

Thanks again for the feedback.

-- 
Brett Lymn
--
Sent from my NetBSD device.

"We are were wolves",
"You mean werewolves?",
"No we were wolves, now we are something else entirely",
"Oh"