Potential static analysis ideas

[Dan Carpenter <dan.carpenter@oracle.com>]

Rust has many good static analysis features but if we wanted we could
implement a number of stricter rules in C.  With Smatch I have tried to
focus on exclusively on finding bugs because everyone can agree that
bugs are bad.  But if some subsystems wanted to implement stricter rules
just as a hardenning measure then that's a doable thing.

For example, I've tried a bunch of approaches to warning about when the
user can trigger an integer overflow.  The challenge is that most
integer overflows are harmless and do not cause a real life bug.

I've also written some checks for locking bugs where the warning is
"expected lock 'foo' held when accessing struct member 'bar'".  The
problem is that I'm not a locking expert.  My process is 1) Infer the
pairing between this lock protect this pointer.  2)  Make list of places
where the pointer holds a function pointer and we free it under lock
(I still need to add the locking part).  3) Make a list of places where
we access the data without holding the lock and the attacker can control
the timing with userfaultd or whatever.

This sort of locking check which finds real life exploitable bugs is
much more complicated to write.  If there were subsystems which had
simpler, hardenned rules then that would be easier.

Another idea is that if I were a subsystem maintainer, I think I would
enforce my prefered error handling rules.  1) Every allocation function
must have exactly one free function.  2) Every function should clean up
after itself on failure. 3) Never free things that aren't allocated.
4) The frees must happen in reverse order from how they were allocated.

These days we have disabled GCC's uninitialized warnings so it falls
to static analysis devs to review all the new warnings.  I sometimes
ignore warnings if they look like probably they aren't a bug.  Does this
function allow zero size writes?  Does this switch statement really need
a default case?  Maybe sometimes I miss bugs.

Here is another example of something which I have a check for but I
haven't pushed.

	ret = frob();
	if (!ret)
		return ret;

This code is normally correct but sometimes it means the if statement is
reversed and it should be "if (ret) return ret;".  To me returning a
literal is always more clear but but clearly the original author felt
"ret" was more clear...  It's only a few bugs per year so it's not a big
deal either way.

A sort of related example that I have pushed is this:

	int ret = 0;

	/* 20 lines of code */

	if (condition)
		goto cleanup;

These trigger a published Smatch warning for missing error codes but I
only email people when I can't understand the code.  Please, put the
"ret = 0;" within 4 lines of the goto.

Those are just some ideas of things we could do if we wanted to use
static analysis for hardenning instead of for fixing bugs.

regards,
dan carpenter