[sanesecurity] Re: [PATCH] Why was this Pattern File not updated?

  • From: Loïc Le Loarer <lll+sane@xxxxxxx>
  • To: sanesecurity@xxxxxxxxxxxxx
  • Date: Wed, 21 Jan 2009 21:57:12 +0100

Hi,

On Wednesday January 21 2009 at 09:15:07 PM +0100, Bruno De Fraine wrote:
> On 21-jan-09, at 19:20, Loïc Le Loarer wrote:
>
>> The problem is that the cp command which "update" the database from
>> /var/cache/sanesecurity/phish.ndb to
>> /var/lib/clamav/sanesecurity-phish.ndb doesn't preverve the time. So the
>> destination takes the date where the update happens.
>>
>> If the mirror is long to take the new file, the new file can have a date
>> which is older than the time at which you launch the update script. And
>> as the two files dates are compared before the the final copy, the problem 
>> happens.
>
> This is well-spotted. It is not sane that the file in $cache_dir retains 
> the original timestamp, while the file in $data_dir receives the 
> timestamp of updating, and the two timestamps are compared.
>
>> Here is a patch which use rsync instead of cp for the final copy, it
>> preverves the time and do the copy atomically (because rsync do), which
>> avoid any problem if clamav reload the database during the cp.
>
> To preserve the timestamp of a local copy, the simplest solution would  
> be "cp -a".

True, or better cp --preserve=timestamps (we don't need -d and -R and
--preserve=mode,ownership). This is enough to fix the upper bug.

> What makes you conclude rsync copies atomically?

This is the way it works (unless you use the --inplace option). rsync
first create a temporary file for the new version and then do a rename
to the new name. On the contrary, cp is overwritting the existing file,
so if you read the file at the same time cp is running, you see a file
which is a mix of the previous and the new one (or a truncated version
of the new, it depends if cp first truncates, I don't remember). So when
multiple application are using the same files at the same time, cp
should be avoided and an atomic rename should be done.

Instead of using rsync, you can also do :
cp orig tmp
mv tmp final
It has the same effect.

See this extract from the rename system call man page:
--------
int rename(const char *oldpath, const char *newpath);

If  newpath already exists it will be atomically replaced (subject
to a few conditions; see ERRORS below), so that there is no point
at which another process attempting to access newpath will find it
missing.
--------
See also this from rsync man page:
--------
--inplace:
WARNING:  The  file's  data  will be in an inconsistent state
during the transfer (and possibly afterward if  the transfer
gets  interrupted),  so  you  should  not  use this option to
update files that are in use.  Also note that rsync  will be
unable  to update a file in-place that is not writable by the
receiving user.
----------

> Also, your 
> patch changes the current directory, but I don't think that is necessary 
> ($db includes a full path).

I don't know why yet but the command 'rsync --perms --times "$db"
"$clamd_dbdir/sanesecurity-$db_name"' fails if rsync doesn't have write
access to the current working dirrectory (this is what I tryed to
explain in the comment of the 'cd' command). It is strange and I haven't
find out why yet.

Anyway, for a script which is launch from cron, it isn't a bad idea to
change current working directory to a known location...

Best regards.
-- 
Loïc

Other related posts: