illumos gate

From the Joyent bug report.

What's happening here is obvious enough. We're dying because iconv(3c) is being passed a first argument of 0xffffffffffffffff or -1, which we interpret as an iconv_t which is a pointer type:

> ::status
debugging core file of .oui (64-bit) from lg-db03.lv.livegamer.com
initial argv: 
/home/oracle/Downloads/database/install/.oui -ignorePrereq -silent -responseFil
threading model: native threads
status: process terminated by SIGSEGV (Segmentation Fault), addr=
ffffffffffffffff
> ::dis
mdb: failed to read instruction at 0: no mapping for address
> $C
fffffd7fffdf09a0 libc.so.1`iconv+9()
fffffd7fffdf2eb0 nls_open+0xb28()
fffffd7fffdff890 main_helper+0x3d0()
fffffd7fffdff8a0 0x404a6c()
> libc.so.1`iconv+9::dis
libc.so.1`iconv:                pushq  %rbp
libc.so.1`iconv+1:              movq   %rsp,%rbp
libc.so.1`iconv+4:              testq  %rdi,%rdi
libc.so.1`iconv+7:              je     +0xf     <libc.so.1`iconv+0x18>
libc.so.1`iconv+9:              movq   (%rdi),%r9
libc.so.1`iconv+0xc:            movq   0x18(%r9),%rdi
libc.so.1`iconv+0x10:           xorl   %eax,%eax
libc.so.1`iconv+0x12:           call   *0x8(%r9)
libc.so.1`iconv+0x16:           jmp    +0x14    <libc.so.1`iconv+0x2c>
libc.so.1`iconv+0x18:           xorl   %eax,%eax
libc.so.1`iconv+0x1a:           call   -0xdc7f  <libc.so.1`___errno>
libc.so.1`iconv+0x1f:           movl   $0x9,(%rax)
libc.so.1`iconv+0x25:           movq   $-0x1,%rax       <0xffffffffffffffff>
libc.so.1`iconv+0x2c:           leave  
libc.so.1`iconv+0x2d:           ret

size_t
iconv(iconv_t cd, const char **inbuf, size_t *inbytesleft,
        char **outbuf, size_t *outbytesleft)
{
        /* check if cd is valid */
        if (cd == NULL) {
                errno = EBADF;
                return ((size_t)-1);
        }

        /* direct conversion */
        return ((*(cd->_conv)->_icv_iconv)(cd->_conv->_icv_state,
            inbuf, inbytesleft, outbuf, outbytesleft));
}

So the customer's function, nls_open(), is passing (iconv_t)-1 to iconv(), which causes the program to abort. This is a bug in the customer's program. However, it also appears that we should be checking for -1 here in addition to NULL, since that's the error sentinel that iconv_open(3c) et al return. There's also various code in the customer's binary that seems to be checking iconv_open() returns against -1, so it's not clear why they're getting here. We'd need the source code to debug that.

The obvious solution here is probably to add a check for -1 and return EBADF in that case as well. That should allow the customer's program to avoid dumping core, although it's unclear that will really make any difference since the customer's program doesn't bother to check iconv()'s return code:

nls_open+0xb1c:                 leaq   0xffffffffffffdb68(%rbp),%r8
nls_open+0xb23:                 call   -0x17420 <PLT=libc.so.1`iconv>
nls_open+0xb28:                 leaq   0xffffffffffffebd0(%rbp),%rdi
nls_open+0xb2f:                 call   -0x1781c <PLT=libc.so.1`strlen>
nls_open+0xb34:                 movq   %rax,%rdi
nls_open+0xb37:                 incq   %rdi
nls_open+0xb3a:                 call   -0x177e7 <PLT=libc.so.1`malloc>

It's entirely possible that the customer's program is simply assuming that some subset of iconv modules is present and cannot handle their absence, although I can't really be sure of that here. Since iconv_open() is documented to fail, their code needs to handle these cases; it also needs to check the return code from iconv() prior to using the converted strings. Shipping more iconv modules (which is a good thing to do in any case) may or may not hide the customer's bug.

In the meantime, we can try changing libc not to abort when given (iconv_t)-1 here. Indeed, it seems that we should, though this code has not changed in at least a decade. It's likely that the customer's broken program will only crash somewhere else or produce bogus output.