[dev.icinga.com #5686] SEGV in idomod syslog call on Solaris

[icinga-migration]

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

Assignee: Tommi
Status: Resolved (closed on 2014-03-10 11:39:04 +00:00)
Target Version: 1.12
Last Update: 2014-12-08 14:47:02 +00:00 (in Redmine)

Icinga Version: 1.10.0
OS Version: any

---

Reading icinganebmodEsaaU0
t@1 (l@1) signal SEGV (no mapping at the fault address) in strlen at 0xffffffff7eb3bdf0
0xffffffff7eb3bdf0: strlen+0x0050:      ld       [%o2], %o1
Current function is idomod_open_debug_log
 4593                   syslog(LOG_ERR, "Warning: Could not open debug file '%s' - '%s'", idomod_debug_file, strerror(errno));
(dbx) where
current thread: t@1
  [1] strlen(0x0, 0x53, 0x0, 0x0, 0x0, 0x53), at 0xffffffff7eb3bdf0
  [2] _ndoprnt(0xffffffff7ffe5ec6, 0xffffffff7ffe6b60, 0xffffffff7eba9e80, 0xffffffff7ffe52f9, 0x0, 0xffffffff7ffe5ec5), at 0xffffffff7ebabbf0
  [3] vsnprintf(0xffffffff7ffe6309, 0x0, 0xffffffff7ffe5ea0, 0xffffffff7ffe6b60, 0xff0000, 0xffffffff7ffe5ea0), at 0xffffffff7ebae3cc
  [4] _vsyslog(0xe83383cf000dcfc5, 0xffffffff7ffe62a0, 0xffffffff7ffe6b60, 0x25, 0x13c0, 0x27), at 0xffffffff7eb784d4
  [5] _syslog(0x3, 0xffffffff7dc1f3c0, 0x0, 0xffffffff7ebdc687, 0x0, 0xffffffff7ed3e000), at 0xffffffff7eb7801c
=>[6] idomod_open_debug_log(), line 4593 in "idomod.c"
  [7] idomod_log_debug_info(level = 1, verbosity = 2, fmt = 0xffffffff7dc1f3f0 "idomod_open_debug_log()\n", ... = 0x1003a93dc, ...), line 4668 in "idomod.c"
  [8] idomod_open_debug_log(), line 4597 in "idomod.c"
  [9] idomod_init(), line 189 in "idomod.c"
  [10] nebmodule_init(flags = 0, args = 0x1003aabf0 "config_file=idomod-oracle.cfg", handle = 0x1003a9150), line 136 in "idomod.c"

Solaris syslog call doesnt like nullpointer argments because of internal vsnprintf funtion
In this particular case debug was activated, but no debug file supplied. therefore idomod_debug_file is a null pointer.
Solution: use icinga supplied gnu printf before calling syslog to handover a proper string

Changesets

2014-03-09 20:40:09 +00:00 by crfriend 2c794f0

Fix "SEGV in idomod syslog call on Solaris" #5686

   This patch adds a check for a null-pointer to the filename
string to circumvent segfaults for Solaris.  Since the call can
never complete normally as invoked, return ERROR status and bail.

Fixes: #5686

2014-03-09 21:44:25 +00:00 by crfriend 38a5fd1

Fix "SEGV in idomod syslog call on Solaris".

This patch adds a check for a null-pointer to the filename
string to circumvent segfaults for Solaris.  Since the call can
never complete normally as invoked, return ERROR status and bail.

Fixes #5686

[icinga-migration]

Updated by crfriend on 2014-03-07 01:59:50 +00:00

Tommi wrote:

Solution: use icinga supplied gnu printf before calling syslog to handover a proper string

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?

[icinga-migration]

Updated by Tommi on 2014-03-08 19:37:11 +00:00

• Target Version set to 1.12

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.

[icinga-migration]

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. ;-)

[icinga-migration]

Updated by crfriend on 2014-03-10 11:39:04 +00:00

• Status changed from Assigned to Resolved
• Done % changed from 0 to 100

Applied in changeset icinga-core:2c794f06427d9aeaf73db5313ae25c3b26bf7349.

[icinga-migration]

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.

[icinga-migration]

Updated by crfriend on 2014-03-10 20:04:33 +00:00

Tommi wrote:

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.

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.)

[icinga-migration]

Updated by mfriedrich on 2014-12-08 14:47:02 +00:00

• Project changed from 18 to Core, Classic UI, IDOUtils
• Category changed from 71 to IDOUtils
• Icinga Version changed from 1 to 1
• OS Version set to any