Package: minidlna
Version: 1.3.3+dfsg-1.1
Tags: patch

In two places in minidlna.c, there's a library call to system() to
delete files in minidlna's configured cache directory (by default
/var/cache/mindlna), via code of the form

    snprintf(cmd, sizeof(cmd), "rm -rf %s/files.db %s/art_cache", db_path, 
db_path);
    if (system(cmd) != 0)
        DPRINTF(E_FATAL, L_GENERAL, "Failed to clean old file cache!  
Exiting...\n");

One instance of this code, in init(), is run in response to the -R
command line option. The other, in check_db(), is run whenever minidlna
has to build a new files.db from scratch, either because it's being run
for the first time, or because an old cache was incompatible with a new
version, or because the -R option deleted the old version.

The instance of this code in check_db() can fail due to a signal
handling goof, because by that time two other things have happened:

 1. init() has installed process_handle_child_termination() as the
    handler for SIGCHLD, which indiscriminately calls waitpid(-1, ...)
    to reap any available child processes

 2. a second thread has been started in the process, which according to
    gdb is being done by blas_thread_init() in libopenblas (!).

glibc's implementation of system() blocks SIGCHLD while it's executing,
so that a SIGCHLD handler can't wait for its subprocess before it does.
But apparently that only blocks it in the current thread. Watching the
failure with a bpftrace script, the _second_ thread is receiving the
SIGCHLD and reaping the return code of the 'rm' command, before glibc's
wait call can do it. So the wait fails with ECHILD; system() reports
failure; minidlna aborts at startup.

This is a race condition, so it doesn't always happen. In particular,
I've found it depends on other conditions on the host OS, and also,
running under strace can perturb away the problem.

Additionally, the snprintf call shown above is careless in how it
constructs the rm command: the cache directory pathname from the
configuration file is used without quoting. It works OK as long as the
cache directory is the default /var/cache/minidlna, which contains no
spaces or other special shell characters. But it's a latent bug.

I attach a patch which solves both problems, by abandoning the strategy
of system("rm -rf stuff") completely, replacing it with an in-process
recursive deletion function.

Cheers,
Simon

-- 
import hashlib; print((lambda p,q,g,y,r,s,m: (lambda w:(pow(g,int(hashlib.sha1(
m.encode('ascii')).hexdigest(),16)*w%q,p)*pow(y,r*w%q,p)%p)%q)(pow(s,q-2,q))==r
and s%q!=0 and m)(12342649995480866419, 2278082317364501, 1670428356600652640,
5398151833726432125, 645223105888478, 1916678356240619, "<ana...@pobox.com>"))

Attachment: 0001-Replace-rm-rf-with-in-process-deletion-code.patch
Description: Binary data

Reply via email to