[PATCH] Samba VirusFilter (version 12)

[Trever L. Adams]

On 01/02/2018 04:26 PM, Jeremy Allison wrote:
> On Tue, Jan 02, 2018 at 10:34:47AM -0700, Trever L. Adams via samba-technical wrote:
>> Thank you to all who have been most helpful it getting things cleaned
>> up. Hopefully this is ready for merging.
> Will still need a few README.Coding changes, but closer !
>
> Can you explain what this code does ?
>
>                 q_filepath = talloc_asprintf(talloc_tos(), "%s/%s%s%s", q_dir,
>                                              q_prefix, base_name, q_suffix);
>
>                 TALLOC_FREE(q_dir);
>                 TALLOC_FREE(q_prefix);
>                 TALLOC_FREE(q_suffix);
>
>                 become_root();
>
>                 q_smb_fname = synthetic_smb_fname(mem_ctx, q_filepath,
>                                                   smb_fname->stream_name, NULL,
>                                                   smb_fname->flags);
>                 if (q_smb_fname == NULL) {
>>>>>>>>>>>>>>           unlink(q_filepath);
>                         unbecome_root();
>                         return VIRUSFILTER_ACTION_DO_NOTHING;
>                 }
>
>                 int_ok = virusfilter_vfs_next_move(handle, smb_fname,
>                                                    q_smb_fname);
>                 if (int_ok == -1)
>                 {
>                         unbecome_root();
>                         DBG_ERR("Rename failed: %s/%s: Rename failed: %s\n",
>                                 cwd_fname, fname,
>                                 strerror(errno));
>                         return VIRUSFILTER_ACTION_DO_NOTHING;
>                 }
>                 unbecome_root();
>
> Why are you unlinking q_filepath under root protections here ?
>
> Once I understand that, I'll send you a README.coding fix
> you can merge in and then I'll do a final review.
>
> Thanks !
>
> Jeremy.
>
Jeremy,

I am very sorry. This was a mistake from when I needed a safe call when
debugging a memory related crash over a year ago. It some how slipped
into my commits back then. You called me on it before. As noted
elsewhere I went back to a previous version of this (quarantine) code in
a recent changeset to simplify things and make the quarantine
functionality work like more people might expect it to. Unfortunately,
while integrating changes from the then current version into the old
one, this bug slipped back in.

The unlink is supposed to be TALLOC_FREE. I have changed it locally.
Again, my apologies for this slip. Thank you for catching it a second time.

I am sorry that I am missing other coding style issues. I look forward
to merging your changes.

Trever


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 886 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180102/b10f4ee4/signature.sig>