Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.Sign up
[dev.icinga.com #5686] SEGV in idomod syslog call on Solaris #1432
This issue has been migrated from Redmine: https://dev.icinga.com/issues/5686
Created by Tommi on 2014-02-21 10:54:02 +00:00
Solaris syslog call doesnt like nullpointer argments because of internal vsnprintf funtion
2014-03-09 20:40:09 +00:00 by crfriend 2c794f0
2014-03-09 21:44:25 +00:00 by crfriend 38a5fd1
Updated by crfriend on 2014-03-07 01:59:50 +00:00
With all due respect, the right thing to do here is to flag the thing as an error stating that the filename isn't defined and then bail. Solaris gets it right here where GNU gets it wrong (and actually I'm rather surprised that fopen() doesn't cause the segfault).
In this case where null-pointers get passed around (which is "safe" (but stupid) in GNU-land) they really represent portability errors and should be so treated. The null-pointer should have been trapped before passing to fopen() and fopen() never having been called in the first place.
Shall I craft a portable patch?
Updated by Tommi on 2014-03-08 19:37:11 +00:00
The problem seams to be the null pointer string in the solaris version of the vprintf used by syslog call raised in every case a segv, not the fopen call with a null pointer filename. We had already several similar issues. I tried to replaced it by a real empty string. But there are still a lot of occurences of using syslog left, where the parameters are not checked. if you have a better solution, you are welcome.
Updated by crfriend on 2014-03-09 20:30:33 +00:00
The update to #5686 proper is trivial, but that does leave the entire morass of null-pointer problems left unsolved -- and, possibly, unsolvable unless we want to do explicit checking on every blasted string-pointer going.
I'll have something for #5686 in a little bit. Since the call can only fail as a valid file-name wasn't supplied this'll be an easy one.
Catching all the others? I'd need to take a cue from Hercules and practice my dam-busting skills. ;-)
Updated by Tommi on 2014-03-10 19:22:21 +00:00
I was more thinking about to write our own syslog function which should take the same parameters, but uses the icinga supplied nullpointer safe Gnu asprintf function to build a real string first and pass the buffer as single %s argument to the original syslog system call. but i am not know how to overload the orignal syslog call first and use them afterwards again. Otherwise you must search and replace all syslog call with a new Name, which will be a huge change.
Updated by crfriend on 2014-03-10 20:04:33 +00:00
Even though it can create more work correcting the problem I remain philosophically opposed to writing "wrappers" that mask (and, worse, thereby encourage) blatant errors like null-pointers. Yes, they can be cheap "band-aids" but can cause really nasty down-stream problems unless everybody downstream knows that they're in place and what they produce. It's for this reason that I find the FSF's (and SGI's) notion of returning a valid pointer to a string of "(null)" in place of a null-pointer pernicious, and I'm on public record griping about it. Simply put, I'm the sort of guy who wants to fix the root of the problem and not merely mask a symptom.
Ferretting out every piece of such code in something the size of Icinga, which has been contributed to by so many people -- some of whom are professional programmers and some of whom are not -- would be akin to cleaning out the Augean Stables. Fortunately, or not, depending what side of the fence one is on, it's mostly the non-Linux folks who get the fast-and-furious (and fatal) segfault in response to these matters; the FSF/GNU folks get files named strange things put in strange places (and other occasional oddities). So, given the relative maturity of the code, it's actually easier to root-cause-analyze these things when they come in and fix them individually rather than a top-down overhaul.
(The last root-cause piece I worked on resulted in a 1400-line change to the code-base and introduced a couple of other bugs thanks to typos, which, whilst quickly fixed remain an embarrassment.)