Hi,
> > In particular, one question that still would be needed to be answered
> > is whether the current umask should have any effect on the permissions
> > of created files, as it currently does (partially).
>
> The created file in this instance is a compressed version of the
> original file, with the same permissions and ownership.
In particular in that very case, it is not, at least not in Sarge. The
mode of the source file is "applied" to the destination file in the open()
call only, and that is affected by the current umask.
I guess this should stay backwards-compatible?!
> A patch would be most welcome.
Attached you find a patch for 3.7. It's untested, though. I don't have
any clue what that SE-Linux stuff does, so the changes might be wrong
with regard to that ...
Florian
diff -urN logrotate-3.7.orig/logrotate.c logrotate-3.7/logrotate.c
--- logrotate-3.7.orig/logrotate.c 2006-09-21 14:02:34.000000000 +0200
+++ logrotate-3.7/logrotate.c 2006-09-22 16:56:23.000000000 +0200
@@ -148,16 +148,33 @@
fstat(inFile, &sb);
- if ((outFile = open(compressedName, O_RDWR | O_CREAT | O_TRUNC,
sb.st_mode)) < 0) {
- message(MESS_ERROR, "unable to open %s for compressed output\n",
- compressedName);
+ if (unlink(compressedName) && errno != ENOENT) {
+ message(MESS_ERROR, "unable to make sure %s doesn't exist (unlink
failed): %s\n",
+ compressedName, strerror(errno));
+ close(inFile);
+ return 1;
+ }
+
+ if ((outFile = open(compressedName, O_RDWR | O_CREAT | O_EXCL, 0)) < 0) {
+ message(MESS_ERROR, "unable to open %s for compressed output: %s\n",
+ compressedName, strerror(errno));
close(inFile);
return 1;
}
if (fchown(outFile, sb.st_uid, sb.st_gid)) {
- message(MESS_ERROR, "unable to change owner of output file %s\n",
- compressedName);
+ message(MESS_ERROR, "unable to change owner of output file %s: %s\n",
+ compressedName, strerror(errno));
+ close(inFile);
+ close(outFile);
+ return 1;
+ }
+
+ int um=umask(0777);
+ umask(um);
+ if (fchmod(outFile, sb.st_mode & ~um)) {
+ message(MESS_ERROR, "error setting mode of %s: %s\n",
+ compressedName, strerror(errno));
close(inFile);
close(outFile);
return 1;
@@ -293,7 +308,13 @@
}
}
#endif
- fdsave = open(saveLog, O_WRONLY | O_CREAT | O_TRUNC,sb->st_mode);
+ if (unlink(saveLog) && errno != ENOENT) {
+ message(MESS_ERROR, "unable to make sure %s doesn't exist (unlink
failed): %s\n",
+ saveLog, strerror(errno));
+ close(fdcurr);
+ return 1;
+ }
+ fdsave = open(saveLog, O_WRONLY | O_CREAT | O_EXCL,0);
#ifdef WITH_SELINUX
if (selinux_enabled) {
setfscreatecon(prev_context);
@@ -309,13 +330,6 @@
close(fdcurr);
return 1;
}
- if (fchmod(fdsave, (S_IRUSR | S_IWUSR) & sb->st_mode)) {
- message(MESS_ERROR, "error setting mode of %s: %s\n",
- saveLog, strerror(errno));
- close(fdcurr);
- close(fdsave);
- return 1;
- }
if (fchown(fdsave, sb->st_uid, sb->st_gid)) {
message(MESS_ERROR, "error setting owner of %s: %s\n",
saveLog, strerror(errno));
@@ -655,18 +669,12 @@
"gid = %d\n", (unsigned int)createMode, (int)createUid,
(int)createGid);
if (!debug) {
- fd = open(log->files[logNum], O_CREAT | O_RDWR, createMode);
+ fd = open(log->files[logNum], O_CREAT | O_RDWR | O_EXCL, 0);
if (fd < 0) {
message(MESS_ERROR, "error creating %s: %s\n",
log->files[logNum], strerror(errno));
hasErrors = 1;
} else {
- if (fchmod(fd, (S_IRUSR | S_IWUSR) & createMode)) {
- message(MESS_ERROR, "error setting mode of "
- "%s: %s\n", log->files[logNum],
- strerror(errno));
- hasErrors = 1;
- }
if (fchown(fd, createUid, createGid)) {
message(MESS_ERROR, "error setting owner of "
"%s: %s\n", log->files[logNum],
