Bug 411441 - cronolog -l fails to update its symlink when log is deleted or too big
cronolog -l fails to update its symlink when log is deleted or too big
Status: NEW
Product: Fedora EPEL
Classification: Fedora
Component: cronolog (Show other bugs)
el5
All Linux
low Severity high
: ---
: ---
Assigned To: jafo-redhat
Fedora Extras Quality Assurance
http://www.cronolog.org/
ActualBug
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-12-04 21:57 EST by Arenas Belon, Carlo Marcelo
Modified: 2011-12-28 03:26 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 1279952 (view as bug list)
Environment:
Last Closed:
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
removes stat calls (which can fail) and use unchecked calls directly (666 bytes, patch)
2007-12-04 21:57 EST, Arenas Belon, Carlo Marcelo
no flags Details | Diff
remove stat calls for the links and use unlink directly (628 bytes, patch)
2007-12-06 11:28 EST, Arenas Belon, Carlo Marcelo
no flags Details | Diff

  None (edit)
Description Arenas Belon, Carlo Marcelo 2007-12-04 21:57:16 EST
Description of problem:
when cronolog is used with the -l (or equivalents like -H and -S) it won't be
able to update that link if any of the following common situations happen :

1) log file the link is pointing to gets deleted
2) log file the link is pointing to is bigger than 2GB in 32bit environments

Version-Release number of selected component (if applicable):
1.6.2-6 (or any other previous release)

How reproducible:
Always


Steps to Reproduce:
1. start cronolog with -l
2. either delete the linked file (not the link) or wait until it goes over 2GB
in a 32bit architecture
3. wait for cronolog to try to rotate the log
  
Actual results:
link is never updated until process is restarted and link manually deleted

Expected results:
link should be updated normally

Additional info:
first case reported upstream and has a proposed fix (now upstream site is down
so it can't be retrieved from there) which was meant to be included in the next
release.

the patch is available as part of the gentoo package for cronolog and a cache of
the report can be retrieved from google from :

http://209.85.173.104/search?q=cache:AxeDmxZhraUJ:cronolog.org/patches/cronolog-missing-symlink-patch.txt+cronolog-missing-symlink-patch.txt&hl=en&ct=clnk&cd=2&gl=us&client=firefox-a

since the affected function doesn't have any valid error detection and the
proposed patch doesn't support case 2, a different fix is proposed which has
also the added benefit of protecting against similar failures when using the -P
parameter
Comment 1 Arenas Belon, Carlo Marcelo 2007-12-04 21:57:16 EST
Created attachment 277591 [details]
removes stat calls (which can fail) and use unchecked calls directly
Comment 2 Arenas Belon, Carlo Marcelo 2007-12-05 14:17:17 EST
Comment on attachment 277591 [details]
removes stat calls (which can fail) and use unchecked calls directly

disregard this patch, the logic got mixed up by excesive refactoring, but the
principle is still valid (stat calls are not valid when testing for the
existence of symlinks and since there is no error checking they are irrelevant
anyway when unlink could be called instead and the error ignored)
Comment 3 Arenas Belon, Carlo Marcelo 2007-12-06 11:28:31 EST
Created attachment 279871 [details]
remove stat calls for the links and use unlink directly

essentially the same patch posted before, and even if it looks confusing
because of the unified patch format has been validated to be correct with all
combination of affected flags "-S, -H, -l, -P" and for both cases reported.

upstreamed patch (or suggestion for a patch) also covers both cases out of luck
and the fact that there is no error checking in that function, but the logic is
incorrect as it still stats the linked file and not the link for prevlinkname
(-P) and relies on the fact that even when that fails the link is overwritten
by the following rename call.

for a test case do (in a system with more than 2GB of memory, or a very fast
disk with ample free disk space)

  cat /dev/zero | cronolog -l log %Y%m%d%H%M.log
Comment 4 Arenas Belon, Carlo Marcelo 2007-12-18 00:11:45 EST
upstream is back online with the following being a report of this bug which was
never resolved (from the "Known Bugs" page in http://cronolog.org/bugs.html) :

http://cronolog.org/mailing-list/msg00259.html

and this being the proposed fix which is the one that was referenced on that old
cached record :

http://cronolog.org/patches/cronolog-missing-symlink-patch.txt
Comment 5 Arenas Belon, Carlo Marcelo 2007-12-18 00:29:35 EST
the jumbo patch (all patches are linked into the "patches" page in
http://cronolog.org/patches/index.html) and that was meant to be part of release
1.7.0 which never got out of beta, has a different implementation of this
solution which extends the original fix to also uses lstat for prevlinkname
(because using stat is logically incorrect as I mentioned earlier) and as can be
seen in :

http://cronolog.org/patches/cronolog-jumbo-patch.txt

a similar fix was re-implemented while looking for a solution to this bug and
later discarded and replaced by the proposed fix, because even though it adds a
little more error checking and is logically correct it is not portable (and
portability for it wasn't checked at "configure" time) and the final solution is
more complex and still missing error checking for the other calls (rename and
unlink) and therefore not complete.

Note You need to log in before you can comment on or make changes to this bug.