Hi Nigel,

First, I wanted to thank you VERY MUCH, for getting this port into an
active project which is integrated with the main clamAV source.  I had
given up hope of this.  

I considered working with the cygwin environment as a compromise to
getting a native port.  This didn't work for me, since I tend to often
work with a snapshot and need to build from sources.  Building clamav
from source on cygwin works and doesn't work from one week/month to the
next depending on magical internal cygwin changes which affect libtool
behavior and are beyond my level of concern.

Since I had given up hope, I had resorted to running clamav (clamd
really) on a Fedora linux environment hosted inside a Virtual Machine
running on the desired systems.  This is a somewhat steep hill to climb,
but once configured can readily work with the mainstream code base.

So, back to the w32 port:

  1) Can you describe the tool-chain you are working with on windows?
What external packages/libraries have been configured and how were they
built, where are they stored, etc?
  2) It seems that the binary files in the current clamAV.msi were built
without "SUPPORT FOR DIGITAL SIGNATURES", since freshclam reports this.
What are the issues and plans in this area?  Can I help?
  3) Are the files generated in when building clamAV.msi, merely built
from the latest files from cvs.clamav.net?  If not, where can we get the
actual source files?  It seems that there might be a relationship to
contrib/Windows/Projects/ClamAV/patches.  Is this patch being kept up to
date?  Why have some, but not all of the O_BINARY patches been
incorporated in the mainline code?  In any case, some folks have already
reported issues with the mmap() implementation by reporting error
messages which match the ones implemented in the
contrib/Windows/Projects/ClamAV/libclamav/compat.c code.  The attached
patch fixes mmap() to support multiple concurrent mmap() calls, and
provides fixes which will work for both the mmap and readdir code when
compiled for a 64bit OS environment if that ever happens.
  4) freshclam running at the start of the ClamAV service is nice, but
I'd really hope that the system running ClamAV wouldn't reboot or
restart the ClamaAV service very often.  Meanwhile, there have been
virusdb updates for the last few days which have ranged from a minimum
of 4 up to 8 times per day, so clearly continuously running freshclam is
desirable.  What are your plans regarding getting both clamd and
freshclam running in the background?  Can I help?



Nigel Horne wrote:
> 
> Ignasi Prat wrote:
> 
> > Hi Nigel:
> 
> > What I have not been able is to run freshclam in daemon mode:
freshclam
> > -d just exits freshclam with no news.
> 
> That's on my list of things to do.
> >
> > And worse is that while trying clamscan:
> >
> > C:\Program Files\clamAV>clamscan
> > LibClamAV Error: cli_cvdload(): Can't create temporary directory
> > \\clamav-c9e1c8
> > 665a80121bee675b327265a01f
> 
> Try "clamscan --tempdir=c:\windows\temp" or some similar option. It's
on
> my list to ensure the default is set to c:\windows\temp on Windows.

Hmmm...  %WINDIR%\temp doesn't actually exist on a stock windows
environment, some apps create and then use it though.

It would seem to me that keeping all clamAV files, even the temporary
ones, in the same general place is a good idea.  That would suggest that
a temp directory would be appropriate along with conf, data, log and run
directories which are already created.

Using such a directory in a standard place instead of using where (for
instance) the TEMP environment variable points would also help manage
another problem.  Specifically, this problem arises when an "On access
AV scanner" is enabled on the system.  In this case, the clamAV
temporary directory (as well as the data directory) need to described in
an "exclusion" rule for the "On access" scanner.  W32.ClamAV is quite a
ways from providing on access scanning, so an alternate "on access"
scanner should still be the rule.  I've had 2 cases where setting up an
exclusion was necessary:  

  1) some freshclam signatures file updates triggered the on access
scanner.  Clearly false positives, but that this happens requires the
exclusion.
  2) clamd stores incoming STREAM files as temporary files, and when the
content to be scanned contained virus content, the on access scanner
sometimes caught (and deleted it) before clamd had a chance to pass
judgement.

Additionally, since some malware that we're trying to catch, or
generally have an on access scanner to protect against, tend to use
standard temporary directories which should continue to be "on access
scanned" and NOT excluded from such scanning.  So, excluding
WINDIR%\temp would be a bad idea.

---

I recently proposed a patch in clamav-devel with the subject of: "Patch
to allow spaces in file and directory names in clamd/freshclam config
files".

I noticed the need for this when I first took a look at your release on
w32.clamav.net.  I had tried to change file path values in
freshclam.conf and clamd.conf, but the changes didn't have the desired
effect since the config file parser was using a space as a token
delimiter.

This patch, or something like it, would seem to be necessary for ClamAV
on windows, and it doesn't seem to have negative consequences for
non-windows platforms since posix file/directory names can contain
spaces as well.

Aside from the above, I'm interested to know what tool you used to
create the .MSI packages you are currently distributing? I'd like to
take a shot at enriching the installer to create or update clamd.conf
and freshclam.conf with correct options.  For example:
  1) freshclam.conf should specify the Database Mirror with the correct
country code.  
  2) Any path values should reflect the path where the ClamAV.msi
installed to.
  3) It would be nice if commonly switched options could be set/changed
by the install script.
Adding this functionality to the installer might not be hard and would
eliminate or defer the need for a ClamAV control panel applet.

Thanks again for getting this going.

- Mark Pizzolato

diff -c ./compat.c ./fixed/compat.c
*** ./compat.c  Fri Aug  4 18:30:04 2006
--- ./fixed/compat.c    Thu Sep 14 07:50:10 2006
***************
*** 77,86 ****
  
        sprintf(mask, "%s\\*", ret->dir_name);
  
!       ret->find_file_handle = (unsigned int)FindFirstFile(mask,
                                    (LPWIN32_FIND_DATA)ret->find_file_data);
  
!       if(ret->find_file_handle == (unsigned int)INVALID_HANDLE_VALUE) {
                free(ret->find_file_data);
                free(ret->dir_name);
                free(ret);
--- 77,86 ----
  
        sprintf(mask, "%s\\*", ret->dir_name);
  
!       ret->find_file_handle = FindFirstFile(mask,
                                    (LPWIN32_FIND_DATA)ret->find_file_data);
  
!       if(ret->find_file_handle == INVALID_HANDLE_VALUE) {
                free(ret->find_file_data);
                free(ret->dir_name);
                free(ret);
***************
*** 161,170 ****
  
        sprintf(mask, "%s\\*", dir->dir_name);
  
!       dir->find_file_handle = (unsigned int)FindFirstFile (mask,
                                        (LPWIN32_FIND_DATA)dir->find_file_data);
  
!       if(dir->find_file_handle == (unsigned int)INVALID_HANDLE_VALUE) {
                errno = EIO;
                return;
        }
--- 161,170 ----
  
        sprintf(mask, "%s\\*", dir->dir_name);
  
!       dir->find_file_handle = FindFirstFile (mask,
                                        (LPWIN32_FIND_DATA)dir->find_file_data);
  
!       if(dir->find_file_handle == INVALID_HANDLE_VALUE) {
                errno = EIO;
                return;
        }
***************
*** 248,265 ****
        return 0;
  }
  
! static        HANDLE  h;      /* Not thread safe and only one mmap is 
supported at a time */
  
  caddr_t
  mmap(caddr_t address, size_t length, int protection, int flags, int fd, off_t 
offset)
  {
        LPVOID addr;
! 
!       if(h) {
!               /* FIXME */
!               cli_errmsg("mmap: only one region may be mapped at a time\n");
!               return MAP_FAILED;
!       }
  
        if(flags != MAP_PRIVATE) {
                cli_errmsg("mmap: only MAP_SHARED is supported\n");
--- 248,270 ----
        return 0;
  }
  
! #  include <pthread.h>
! 
! static pthread_mutex_t mmap_mutex = PTHREAD_MUTEX_INITIALIZER;
! 
! static struct mmap_context {
!     struct mmap_context *link;
!     HANDLE h;
!     LPVOID view;
!     size_t length;
! } *mmaps = NULL;
  
  caddr_t
  mmap(caddr_t address, size_t length, int protection, int flags, int fd, off_t 
offset)
  {
        LPVOID addr;
!       HANDLE h;
!       struct mmap_context *ctx;
  
        if(flags != MAP_PRIVATE) {
                cli_errmsg("mmap: only MAP_SHARED is supported\n");
***************
*** 269,282 ****
                cli_errmsg("mmap: only PROT_READ is supported\n");
                return MAP_FAILED;
        }
! 
!       h = CreateFileMapping(_get_osfhandle(fd), NULL, PAGE_READONLY, 0, 0, 
NULL);
! 
!       if(h && (GetLastError() == ERROR_ALREADY_EXISTS)) {
!               cli_errmsg("mmap: ERROR_ALREADY_EXISTS\n");
!               CloseHandle(h);
                return MAP_FAILED;
        }
        if(h == NULL) {
                cli_errmsg("mmap: CreateFileMapping failed - error %d\n",
                        GetLastError());
--- 274,285 ----
                cli_errmsg("mmap: only PROT_READ is supported\n");
                return MAP_FAILED;
        }
!       if(address != NULL) {
!               cli_errmsg("mmap: only NULL map address is supported\n");
                return MAP_FAILED;
        }
+       h = CreateFileMapping((HANDLE)_get_osfhandle(fd), NULL, PAGE_READONLY, 
0, 0, NULL);
+ 
        if(h == NULL) {
                cli_errmsg("mmap: CreateFileMapping failed - error %d\n",
                        GetLastError());
***************
*** 287,315 ****
                CloseHandle(h);
                return MAP_FAILED;
        }
!       /* FIXME hi DWORD (unsigned long) is 0, so this may not work on 64 bit 
machines */
!       addr = MapViewOfFile(h, FILE_MAP_READ, (DWORD)0,
!               ((DWORD)address & 0xFFFFFFFF), length);
  
        if(addr == NULL) {
                cli_errmsg("mmap failed - error %d\n", GetLastError());
                CloseHandle(h);
                return MAP_FAILED;
        }
        return (caddr_t)addr;
  }
  
  int
  munmap(caddr_t addr, size_t length)
  {
!       if(h == NULL) {
                cli_warnmsg("munmap with no corresponding mmap\n");
                return -1;
        }
!       UnmapViewOfFile((LPCVOID)addr);
!       CloseHandle(h);
! 
!       h = NULL;
  
        return 0;
  }
--- 290,351 ----
                CloseHandle(h);
                return MAP_FAILED;
        }
!       addr = MapViewOfFile(h, FILE_MAP_READ, 
!               (DWORD)0, ((DWORD)offset & 0xFFFFFFFF), 
!               length);
  
        if(addr == NULL) {
                cli_errmsg("mmap failed - error %d\n", GetLastError());
                CloseHandle(h);
                return MAP_FAILED;
        }
+       pthread_mutex_lock(&mmap_mutex);
+       ctx = cli_malloc(sizeof(*ctx));
+       if(NULL == ctx) {
+               pthread_mutex_unlock(&mmap_mutex);
+               cli_errmsg("mmap: can't create context block\n");
+               UnmapViewOfFile(addr);
+               CloseHandle(h);
+               return MAP_FAILED;
+       }
+       ctx->h = h;
+       ctx->view = addr;
+       ctx->length = length;
+       ctx->link = mmaps;
+       mmaps = ctx;
+       pthread_mutex_unlock(&mmap_mutex);
        return (caddr_t)addr;
  }
  
  int
  munmap(caddr_t addr, size_t length)
  {
!       struct mmap_context *ctx, *lctx = NULL;
! 
!       pthread_mutex_lock(&mmap_mutex);
!         for(ctx = mmaps; ctx && (ctx->view != addr); ) {
!               lctx = ctx;
!                 ctx = ctx->link;
!       }
!       if(ctx == NULL) {
!               pthread_mutex_unlock(&mmap_mutex);
                cli_warnmsg("munmap with no corresponding mmap\n");
                return -1;
        }
!       if(ctx->length != length) {
!               pthread_mutex_unlock(&mmap_mutex);
!               cli_warnmsg("munmap with incorrect length specified - partial 
munmap unsupported\n");
!               return -1;
!       }
!       if(NULL == lctx)
!               mmaps = ctx->link;
!       else
!               lctx->link = ctx->link;
!       pthread_mutex_unlock(&mmap_mutex);
! 
!         UnmapViewOfFile(ctx->view);
!       CloseHandle(ctx->h);
!       free(ctx);
  
        return 0;
  }
diff -c ./compat.h ./fixed/compat.h
*** ./compat.h  Fri Aug  4 18:30:04 2006
--- ./fixed/compat.h    Thu Sep 14 04:32:04 2006
***************
*** 89,95 ****
  struct DIR {
        char    *dir_name;
        int     just_opened;
!       unsigned int     find_file_handle;
        void    *find_file_data;        /* LPWIN32_FIND_DATA */
  };
  typedef struct        DIR     DIR;
--- 89,95 ----
  struct DIR {
        char    *dir_name;
        int     just_opened;
!       void    *find_file_handle;
        void    *find_file_data;        /* LPWIN32_FIND_DATA */
  };
  typedef struct        DIR     DIR;
_______________________________________________
http://lists.clamav.net/cgi-bin/mailman/listinfo/clamav-win32

Reply via email to