[libvirt] [PATCH v1 01/23] virtlockd: Don't SIGSEGV on SIGUSR1

[John Ferlan]

On 10/12/2015 06:25 AM, Michal Privoznik wrote:
> So we have this mechanism that on SIGUSR1 the virtlockd dumps its
> internal state into a JSON file, reexec itself and the reloads
> the internal state back. However, there's a bug in our
> implementation:
> 
>   (gdb) signal SIGUSR1
>   Continuing with signal SIGUSR1.
>   [Thread 0x7fd094f7b700 (LWP 10602) exited]
>   process 10600 is executing new program: /home/zippy/work/libvirt/libvirt.git/src/virtlockd
>   warning: Could not load shared library symbols for linux-vdso.so.1.
>   Do you need "set solib-search-path" or "set sysroot"?
>   [Thread debugging using libthread_db enabled]
>   Using host libthread_db library "/lib64/libthread_db.so.1".
>   [New Thread 0x7fb28bc3c700 (LWP 14501)]
> 
>   Program received signal SIGSEGV, Segmentation fault.
>   0x00007fb29133d530 in virExpandN (ptrptr=0x70, size=8, countptr=0x68, add=1, report=true, domcode=7, filename=0x7fb29138aeab "rpc/virnetserver.c", funcname=0x7fb29138b680 <__FUNCTION__.15821> "virNetServerAddProgram", linenr=661) at util/viralloc.c:288
>   288         if (*countptr + add < *countptr) {
>   (gdb) bt
>   #0  0x00007fb29133d530 in virExpandN (ptrptr=0x70, size=8, countptr=0x68, add=1, report=true, domcode=7, filename=0x7fb29138aeab "rpc/virnetserver.c", funcname=0x7fb29138b680 <__FUNCTION__.15821> "virNetServerAddProgram", linenr=661) at util/viralloc.c:288
>   #1  0x00007fb29132a267 in virNetServerAddProgram (srv=0x0, prog=0x7fb2915d08b0) at rpc/virnetserver.c:661
>   #2  0x00007fb29131f27f in main (argc=1, argv=0x7fff8f771298) at locking/lock_daemon.c:1445
> 
> Notice the NULL @srv passed to frame 2? Usually, the @srv
> variable is initialized on fresh start. However, in case of
> daemon reload, the code path that is responsible for initializing
> the value was not triggered and therefore we crashed immediately.
> Fix this by always setting the variable.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/locking/lock_daemon.c | 2 ++
>  1 file changed, 2 insertions(+)
> 

ACK - although you may want to considering altering the comment after
the virLockDaemonPostExecRestart call to indicate that getting the need
to get the srv pointer again...


John