Bug 2917 - [review] Failure to remove /tmp/spamd-$$-init for certain settings of auto_whitelist_path (and bayes_path)
Summary: [review] Failure to remove /tmp/spamd-$$-init for certain settings of auto_wh...
Status: RESOLVED FIXED
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: spamc/spamd (show other bugs)
Version: 2.61
Hardware: PC Linux
: P5 normal
Target Milestone: 2.62
Assignee: SpamAssassin Developer Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-01-09 15:09 UTC by Michael Buschbeck
Modified: 2004-01-11 02:29 UTC (History)
0 users


Attachment Type Modified Status Actions Submitter/CLA Status
The patch. patch None Malte S. Stretz [HasCLA]
The Real Patch(tm) without the typo. patch None Malte S. Stretz [HasCLA]
Updated patch patch None Theo Van Dinter [HasCLA]
need to change the error output section too ... patch None Theo Van Dinter [HasCLA]
Show Obsolete (3) Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Buschbeck 2004-01-09 15:09:58 UTC 
Upon starting, spamd fails to remove the directory /tmp/spamd-$$-init if
auto_whitelist_path (or, presumably, bayes_path) are set to write their
respective files into that directory itself rather than its ".spamassassin"
subdirectory using the following configuration setting in local.cf:

  auto_whitelist_path ~/auto-whitelist

spamd writes the following message to the system log:

  Failed to remove /tmp/spamd-10877-init: Could not remove /tmp/spamd-10877-init: Directory not empty

The directory remains in /tmp containing only the files auto-whitelist.dir and
auto-whitelist.dir.
Comment 1 Michael Buschbeck 2004-01-09 15:19:18 UTC 
preload_modules_with_tmp_homedir deletes all files in $tmpsadir before trying to remove $tmpsadir and $tmphome themselves.  It never tries to delete files in $tmphome itself before attempting to remove it.  Certain (valid) configuration settings might leave files in there though.

preload_modules_with_tmp_homedir should either *recursively* delete files and directories starting in $tmphome, or should at least try to delete files in $tmphome *as well* after deleting those in $tmpsadir.
Comment 2 Malte S. Stretz 2004-01-09 15:31:41 UTC 
You're right, I'll have this fixed in a minute :)
Comment 3 Theo Van Dinter 2004-01-09 15:32:07 UTC 
moving to 2.62
Comment 4 Malte S. Stretz 2004-01-09 15:41:36 UTC 
Created attachment 1672 [details]
The patch.

Here it is. Michael, could you please test it?
Comment 5 Malte S. Stretz 2004-01-09 15:43:24 UTC 
I really wonder *why* that patch works for me as there's an obvious typo in 
there...
Comment 6 Malte S. Stretz 2004-01-09 15:53:45 UTC 
Created attachment 1673 [details]
The Real Patch(tm) without the typo.

A wise man once said: You shall not drink and code. And watch out which
repository you test against if you've got several on your disk.
Comment 7 Michael Buschbeck 2004-01-09 17:24:40 UTC 
It works *almost* like a charm.  It works perfectly after fixing the following
buglet that remained from the original code:

  $f = Mail::SpamAssassin::Util::untaint_file_path (
         File::Spec->catfile ($tmpsadir, $f)
       );

Make that

  $f = Mail::SpamAssassin::Util::untaint_file_path (
         File::Spec->catfile ($d, $f)  # $d, not $tmpsadir!
       );

...and everything is fine.  Thanks for the prompt response.  :-)
Comment 8 Theo Van Dinter 2004-01-09 18:38:31 UTC 
Created attachment 1674 [details]
Updated patch

Same patch as 1673, but fixed the issue as stated in the last comment.
Comment 9 Theo Van Dinter 2004-01-09 18:39:09 UTC 
moving to 2.62 again ...
Comment 10 Malte S. Stretz 2004-01-10 05:52:36 UTC 
+1 1674 :)
Comment 11 Theo Van Dinter 2004-01-11 10:22:21 UTC 
applied 1674 to 2.62 and head ...  noticed another section of that function 
that needed changing.  patch forthcoming.
Comment 12 Theo Van Dinter 2004-01-11 10:25:19 UTC 
Created attachment 1678 [details]
need to change the error output section too ...
Comment 13 Malte S. Stretz 2004-01-11 11:07:31 UTC 
-1 1678:  
 
That's not necessary. This routine tries to remove $tmphome so that's what the 
error message complains about if it still exists. 
 
$tmpsadir is always below $tmphome, already covered by $tmphome. Checking for 
$err is also not necessary because $err can only be set if $tmphome still 
exists. OTOH if that dir exists and $err is *not* set, something really weird 
happened (== can't happen), that's why there's the "do something". $err will 
always contain the exact reason why $tmphome could not be removed.
Comment 14 Theo Van Dinter 2004-01-11 11:29:27 UTC 
Ok, good point, I missed that. :(