On Wednesday, December 01, 2004 3:59 PM, Tomasz Kojm wrote:
On Wed, 1 Dec 2004 14:28:20 -0800
"Mark Pizzolato" <[EMAIL PROTECTED]> wrote:

> I'm trying to use clamd via TCP Stream connections to ClamAV under
> cygwin, but under any concurrent load, clamd was logging:
>
>                ERROR: ScanStream: Can't create temporary file.
>
> I tracked this down to clamd/scanner.c's use of tmpfile() which is
> documented as being NOT a threadsafe API.  As it turns out, tmpfile()

That was rather caused by a w32 limitation of tmpfile() and not its
thread safeness. Boguslaw Brandys described the problem here, IIRC.

> is actually threadsafe iff the local implementation of getpid()
> returns a different value for each execution thread (i.e. as it does
> on Linux). Hence, most environments don't see this issue.

Every normal POSIX compliant platform provides reentrant and thread safe
tmpfile().

Solaris man pages for tmpnam(), and tempnam() and tmpfile() suggest that tmpfile() uses tmpnam() and that tmpnam() is not threadsafe.


All tmpfile() calls will be removed in the future, though. We want
libclamav to use a single temporary directory that could be changed
dynamically with cl_settmpdir()

This patch provides the functionality that leverages what has been set by cl_settmpdir()


> While exploring the issue of threadsafety in ClamAV, I dug deeper into
> the other places where temp files are created and used in libclamav.
> I found numerous places where concurrency issues existed, even though

Theoretical races will always be possible, that's why cli_gentemp uses
a cryptographic function to generate temporary names. I don't see real
advantages of your changes.

cli_gentemp() uses a mutex internally, the change I propose closes the race by performing the desired "temp" operations with the mutex held. Looking at what is protected by the mutex in cli_gentemp(), there exists a real likelyhood that collisions will occur on systems which have relatively low resolution updates to the microsecond return from gettimeofday().



The mutex is used to protect non-reentrant unrarlib call and not tmpfile().

> progress which would return success instead.  The need for this mutex
> logic has been eliminated.

This change makes libclamav thread unsafe.

I realized that, and have provided proper serialization and removed a normal path memory leak.


- Mark Pizzolato

_______________________________________________
http://lists.clamav.net/cgi-bin/mailman/listinfo/clamav-devel

Reply via email to