Subject: Re: DIAGASSERT in cryptographic functions.
To: None <tech-userlevel@netbsd.org>
From: Luke Mewburn <lukem@cs.rmit.edu.au>
List: tech-userlevel
Date: 09/17/1999 18:17:00
Frank van der Linden writes:
> On Fri, Sep 17, 1999 at 12:02:11AM +0200, Christoph Badura wrote:
> > ISTR Luke saying something about not wanting to have critical programs
> > dump core on him. One possible solution is to fork() and let the child
> > dump core.
>
> Well, I hope Luke will speak up about this himself, but... If you have
> a critical program, and you link it with the DIAG library, that would
> imply to me that you suspect something is wrong. So it's better to
> have it bail out at that point, leaving evidence, then having it
> continue in an unknown state, which is certainly not what you want
> from a "critical program".
>
> In any case, continuing where it isn't expected, leading to undefined
> behaviour, is never a good thing.
The original reason for not dumping core during my implementation
phase was because if I was too aggressive in adding _DIAGASSERT()s
and my shell starts dumping core, it makes things a bit hard to debug.
The current implementation of _DIAGASSERT just does
fprintf(stderr, "assertion ... failed");
syslog(user.debug, "assertion ... failed");
I plan to make it user configurable (with $DIAGASSERT ?) so that the
user can specify one or more of the following actions to occur, in
the order given in the environment var:
fprintf(stderr, "assertion ... failed");
syslog(user.debug, "assertion ... failed");
abort()
nothing
This is on my list of things `to do'.
Back to the issue of the hash/md routines. Unfortunately, these
provide no way of feeding back to the caller that the arguments are
invalid. There's a few other routines in the libraries with similar
attributes.
I've gone through the library looking for funtions which just `return;'
if the diagnostic checks fail, and added `XXXLUKEM' to them. A diff is
available at:
ftp://ftp.netbsd.org/pub/NetBSD/misc/lukem/diagassert.part2.diffs
Depending on the routine, on of the following should be done:
* replace the return with abort().
The hash/md functions should probably have this done.
* remove the /* XXXLUKEM */ comment; it's ok to return in this
case. I effectively did this for functions which do stuff
like free a list; it really doesn't matter if the top-level
pointer is checked against NULL because the invoker doesn't
care anyway.
Opinions/thoughts?