Call-ins where the return value is a string checked for overflow

Final Release Note

Call-ins where the return value is a string are checked for overflow (where possible) and return an INVSTRLEN error if the return area is not large enough, where previously it did not. Note that for string parameters, use of the ydb_string_t type is highly recommended as it facilitates checking for buffer overflows. A char * type does not facilitate such checks and is best avoided. [#446 (closed)]

Description

Though this issue was found via a golang routine, since the return value copy is done within the YDB engine, the issue is created here. There appears to be no code to check the return value length versus the size of the buffer when returning a string (required by the golang wrapper code at this time). Consequently, all it takes is a small golang routine and a small M routine to cause a sig-11.

Go code:

package main

import (
	"fmt"
	"lang.yottadb.com/go/yottadb"
)

// Test what happens when return value is longer than return buffer.

// Constants
const tptoken uint64 = yottadb.NOTTP

func main() {
	retval, err := yottadb.CallMT(tptoken, nil, 20, "testciret")
	if nil != err {
		panic(err)
	}
	fmt.Println("ciret: Return value:       ", retval)
	fmt.Println("ciret: Return value length:", len(retval))
}

M Code:

; Routine to return a string longer than the expected 20 bytes to our (golang) caller
;
	;quit "X234567890123456789012345"	; Return 25 chars
	quit "X2345678901234567890"		; Return 20 chars

Call-in definition file ($ydb_ci points to this file):

testciret : ydb_string_t * ^testciret()

If we only return the 20 chars that fit in the allocated buffer, it works fine. But uncomment the 25 character one and it explodes:

(gdb) where
#0  __pthread_kill (threadid=<optimized out>, signo=3) at ../sysdeps/unix/sysv/linux/pthread_kill.c:57
#1  0x00007f5c549314b8 in gtm_dump_core () at /Distrib/YottaDB/V994/sr_unix/gtm_dump_core.c:72
#2  0x00007f5c549322c4 in gtm_fork_n_core () at /Distrib/YottaDB/V994/sr_unix/gtm_fork_n_core.c:91
#3  0x00007f5c54924782 in generic_signal_handler (sig=11, info=0x7f5c556963c8 <stapi_signal_handler_oscontext+4296>, 
    context=0x7f5c55696448 <stapi_signal_handler_oscontext+4424>) at /Distrib/YottaDB/V994/sr_unix/generic_signal_handler.c:138
#4  <signal handler called>
#5  0x00007f5c5432bc01 in __GI___libc_free (mem=0x13b59d0) at malloc.c:3123
#6  0x00000000004501b0 in runtime.asmcgocall () at /usr/lib/go-1.10/src/runtime/asm_amd64.s:688
#7  0x00007fffd150e150 in ?? ()
#8  0x000000000044def1 in runtime.exitsyscallfast.func1 () at /usr/lib/go-1.10/src/runtime/proc.go:3039
#9  0x000000000044e9d9 in runtime.systemstack () at /usr/lib/go-1.10/src/runtime/asm_amd64.s:409
#10 0x000000000042ce30 in ?? () at /usr/lib/go-1.10/src/runtime/proc.go:1092
#11 0x0000000000000000 in ?? ()
(gdb)

This is the original thread it failed in. The signal was then forwarded to a different thread where the core was created.

(gdb) thr 5
[Switching to thread 5 (Thread 0x7f5c5084f700 (LWP 18309))]
#0  __clock_nanosleep (clock_id=1, flags=1, req=0x7f5c5084ee00, rem=0x0) at ../sysdeps/unix/sysv/linux/clock_nanosleep.c:48
48      ../sysdeps/unix/sysv/linux/clock_nanosleep.c: No such file or directory.
(gdb) where
#0  __clock_nanosleep (clock_id=1, flags=1, req=0x7f5c5084ee00, rem=0x0) at ../sysdeps/unix/sysv/linux/clock_nanosleep.c:48
#1  0x00007f5c549270ec in generic_signal_handler (sig=11, info=0x7f5c556963c8 <stapi_signal_handler_oscontext+4296>, 
    context=0x7f5c55696448 <stapi_signal_handler_oscontext+4424>) at /Distrib/YottaDB/V994/sr_unix/generic_signal_handler.c:377
#2  0x00007f5c54a9a7d5 in ydb_stm_invoke_deferred_signal_handler () at /Distrib/YottaDB/V994/sr_unix/ydb_stm_invoke_deferred_signal_handler.c:51
#3  0x00007f5c54a9afe8 in ydb_stm_thread (parm=0x0) at /Distrib/YottaDB/V994/sr_unix/ydb_stm_thread.c:109
#4  0x00007f5c5468c6db in start_thread (arg=0x7f5c5084f700) at pthread_create.c:463
#5  0x00007f5c543b588f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
(gdb) f 1
#1  0x00007f5c549270ec in generic_signal_handler (sig=11, info=0x7f5c556963c8 <stapi_signal_handler_oscontext+4296>, 
    context=0x7f5c55696448 <stapi_signal_handler_oscontext+4424>) at /Distrib/YottaDB/V994/sr_unix/generic_signal_handler.c:377
377                     MULTI_THREAD_AWARE_FORK_N_CORE(signal_forwarded);
(gdb) p signal_forwarded
$1 = 1
(gdb)

This was obviously triggered by a golang call but the code that failed to do the checks and copied the return value was C code so the belief is that this is also a C problem and general call-in problem as well. These should all be investigated and tests created for them.

Draft Release Note

Call-ins where the return value is a string are check for overflow (where possible) and return an INVSTRLEN error (-YDB_ERR_INVSTRLEN) if the return area is not large enough. Note that for string parameters, use of the ydb_string_t type is highly recommended as it enables check for buffer overflows. A char * type does not enable such checks and is best avoided.

Edited by K.S. Bhaskar