[PATCH] Samba VirusFilter (version 12)

[Jeremy Allison]

On Tue, Jan 23, 2018 at 06:40:35AM -0700, Trever L. Adams wrote:

> Thank you for these wonderful clean ups. I agree with the change on the
> random string. I did find two problems (shown as deltas between your
> patch set and mine). The first is a coding style and a failure:
> 
>  diff --git a/source3/modules/vfs_virusfilter.c
> b/source3/modules/vfs_virusfilter.c
>  new file mode 100644
> -index 00000000000..eb8ac26665f
> +index 00000000000..8703f2d303b
>  --- /dev/null
>  +++ b/source3/modules/vfs_virusfilter.c
>  @@ -0,0 +1,1454 @@
> @@ -1132,7 +1132,7 @@
>  +     * and becoming root over and over.
>  +     */
>  +    if (config->infected_file_action == VIRUSFILTER_ACTION_QUARANTINE) {
> -+        bool ok = false;
> ++        bool ok = true;
>  +        bool dir_exists;
>  +
>  +        /*
> @@ -1149,7 +1149,7 @@
>  +                          config->quarantine_dir);
>  +        }
>  +        unbecome_root();
> -+        if (ok == false) {
> ++        if (!ok) {
>  +            DBG_ERR("Creating quarantine directory %s "
>  +                "failed with %s\n",
>  +                config->quarantine_dir,
> 
> If it is default to false, the module will never load if the directory
> already exists. This is near the end of virusfilter_vfs_connect.

Oh good catch. Sorry for the screw-up :-).

> The second is a coding style that Jim caught that I had fixed locally
> that didn't make it into the last version. My old version made use of
> ok. Based on changes you made, I figured it was okay to simply return
> the returned value.
> 
>  diff --git a/source3/modules/vfs_virusfilter_utils.c
> b/source3/modules/vfs_virusfilter_utils.c
>  new file mode 100644
> -index 00000000000..2b98431f676
> +index 00000000000..628e0aef99a
>  --- /dev/null
>  +++ b/source3/modules/vfs_virusfilter_utils.c
> -@@ -0,0 +1,1031 @@
> +@@ -0,0 +1,1025 @@
>  +/*
>  +   Samba-VirusFilter VFS modules
>  +   Copyright (C) 2010-2016 SATOH Fumiyasu @ OSS Technology Corp., Japan
> @@ -2713,13 +2713,7 @@
>  +    iov.iov_base = discard_const_p(void, data);
>  +    iov.iov_len = data_size;
>  +
> -+    switch (write_data_iov_timeout(io_h->stream, &iov, 1,
> -+        io_h->io_timeout)) {
> -+    case false:
> -+        return false;
> -+    default:
> -+        return true;
> -+    }
> ++    return write_data_iov_timeout(io_h->stream, &iov, 1,
> io_h->io_timeout);
>  +}
>  +
>  +bool virusfilter_io_writel(

Yes, that's the correct change ! I missed that one. Thanks.

> If these are the right solution (and they do work here), then I
> recommend my attached patch set labeled as version 14.

LGTM. Ralph, can I get a second Team reviewer ?

Cheers,

	Jeremy.