severity 279965 grave
thanks
On Tue, May 24, 2005 at 03:22:58PM +0200, Philipp Hartmann wrote:
> I think, this bug is quite serious, since the behaviour of executing
> postrotate scripts is different in Woody and Sarge.
> I don't see any rationale, why this behaviour was changed in the first
> place. The scripts are trusted ones anyway...
> IMHO, there are two ways, to cope with this issue:
>
> 1) apply the patch, attached to this message
> - it runs the scripts through an explicit shell
Upstream have a different patch (attached). I'm not too sure if I like
it, as it relies on the size of the arguments you can pass to a shell.
Effectively it turns runScript() into:
static int runScript(char * logfn, char * script) {
int rc;
if (debug) {
message(MESS_DEBUG, "running script with arg %s: \"%s\"\n",
logfn, script);
return 0;
}
if (!fork()) {
execl("/bin/sh", "sh", "-c", script, NULL);
exit(1);
}
wait(&rc);
return rc;
}
...which doesn't rely on writing to a file at all.
However, to not break #276172 again, it should be
execl("/bin/sh", "sh", "-c", script, "sh", logfn, NULL);
> 2) mention the changed behaviour at least in NEWS.Debian
> - This is necessary, because upgrading from Woody could
> break log rotation completely.
I'm torn between the three possible ways out of this. Of the three
options (your patch, upstream's patch, and documenting the behaviour), I
tend to prefer the modified version of upstream's patch.
I agree that this bug is more serious than the original reporter
thought, which is why I've changed it to "grave", because it potentially
causes data loss when a user upgrades from woody.
(Note to Lars: please, please don't take this as a signal to do an
instant NMU. This bug requires a bit of thought. I'm going to have to
consult the release managers.)
--
Paul Martin <[EMAIL PROTECTED]>
--- logrotate-3.7.1/logrotate.c.noTMPDIR 2004-10-19 23:41:24.000000000
+0200
+++ logrotate-3.7.1/logrotate.c 2005-02-22 17:20:59.357912308 +0100
@@ -75,10 +75,7 @@
}
static int runScript(char * logfn, char * script) {
- int fd;
- char *filespec;
int rc;
- char buf[256];
if (debug) {
message(MESS_DEBUG, "running script with arg %s: \"%s\"\n",
@@ -86,38 +83,12 @@
return 0;
}
- filespec = buf;
- snprintf(buf, sizeof(buf), "%s/logrotate.XXXXXX", getenv("TMPDIR") ?:
"/tmp");
- fd = -1;
- if (!filespec || (fd = mkstemp(filespec)) < 0 || fchmod(fd, 0700)) {
- message(MESS_DEBUG, "error creating %s: %s\n", filespec,
- strerror(errno));
- if (fd >= 0) {
- close(fd);
- unlink(filespec);
- }
- return -1;
- }
-
- if (write(fd, "#!/bin/sh\n\n", 11) != 11 ||
- write(fd, script, strlen(script)) != strlen(script)) {
- message(MESS_DEBUG, "error writing %s\n", filespec);
- close(fd);
- unlink(filespec);
- return -1;
- }
-
- close(fd);
-
if (!fork()) {
- execlp(filespec, filespec, logfn, NULL);
+ execl("/bin/sh", "sh", "-c", script, NULL);
exit(1);
}
wait(&rc);
-
- unlink(filespec);
-
return rc;
}
