Gitweb:
https://git.exim.org/exim.git/commitdiff/b66fecb428871a3eb274d9370671f1eaf8c5ccec
Commit: b66fecb428871a3eb274d9370671f1eaf8c5ccec
Parent: 3a9262974035b6b8efebffcdf50a2371f3636477
Author: Heiko Schlittermann (HS12-RIPE) <hs@???>
AuthorDate: Mon Feb 4 22:01:36 2019 +0100
Committer: Heiko Schlittermann (HS12-RIPE) <hs@???>
CommitDate: Fri Apr 19 15:12:18 2019 +0200
Intercept chown()/fchown() failure and emit a pointer to the bugreport. Closes 2391
In a specific NFS setup we experienced a failing chown(). As it is not
clear, whether this was due to a misconfiguration or if this may happen in
other environments too, we behave as usual (abort the operation), but
issue a MAIN_LOG and PANIC_LOG entry pointing to this Bugreport.
You're encouraged to contact the developers, if you hit this issue.
---
src/src/dbfn.c | 2 +-
src/src/deliver.c | 6 +++---
src/src/directory.c | 2 +-
src/src/exim.c | 32 ++++++++++++++++++++++++++++++--
src/src/functions.h | 39 +++++++++++++++++++++++++++++++++++++++
src/src/mytypes.h | 1 -
src/src/receive.c | 6 +++---
src/src/spool_out.c | 2 +-
src/src/tls-gnu.c | 2 +-
src/src/transports/appendfile.c | 10 +++++-----
10 files changed, 84 insertions(+), 18 deletions(-)
diff --git a/src/src/dbfn.c b/src/src/dbfn.c
index 336cfe7..5555c71 100644
--- a/src/src/dbfn.c
+++ b/src/src/dbfn.c
@@ -209,7 +209,7 @@ if (created && geteuid() == root_uid)
if (Ustat(filename, &statbuf) >= 0 && statbuf.st_uid != exim_uid)
{
DEBUG(D_hints_lookup) debug_printf_indent("ensuring %s is owned by exim\n", filename);
- if (Uchown(filename, exim_uid, exim_gid))
+ if (exim_chown(filename, exim_uid, exim_gid))
DEBUG(D_hints_lookup) debug_printf_indent("failed setting %s to owned by exim\n", filename);
}
}
diff --git a/src/src/deliver.c b/src/src/deliver.c
index c1396a7..696effd 100644
--- a/src/src/deliver.c
+++ b/src/src/deliver.c
@@ -347,7 +347,7 @@ for (int i = 2; i > 0; i--)
#ifndef O_CLOEXEC
(void)fcntl(fd, F_SETFD, fcntl(fd, F_GETFD) | FD_CLOEXEC);
#endif
- if (fchown(fd, exim_uid, exim_gid) < 0)
+ if (exim_fchown(fd, exim_uid, exim_gid, filename) < 0)
{
*error = US"chown";
return -1;
@@ -367,7 +367,7 @@ for (int i = 2; i > 0; i--)
MSGLOG_DIRECTORY_MODE, TRUE);
}
-*error = US"create";
+*error = US"create or open";
return -1;
}
@@ -7072,7 +7072,7 @@ if (addr_local || addr_remote)
that the mode is correct - the group setting doesn't always seem to get
set automatically. */
- if( fchown(journal_fd, exim_uid, exim_gid)
+ if( exim_fchown(journal_fd, exim_uid, exim_gid, fname)
|| fchmod(journal_fd, SPOOL_MODE)
#ifndef O_CLOEXEC
|| fcntl(journal_fd, F_SETFD, fcntl(journal_fd, F_GETFD) | FD_CLOEXEC)
diff --git a/src/src/directory.c b/src/src/directory.c
index e5b6551..2d4d565 100644
--- a/src/src/directory.c
+++ b/src/src/directory.c
@@ -69,7 +69,7 @@ while (c && *p)
/* Set the ownership if necessary. */
- if (use_chown && Uchown(path, exim_uid, exim_gid))
+ if (use_chown && exim_chown(path, exim_uid, exim_gid))
{ p = US"set owner on"; goto bad; }
/* It appears that any mode bits greater than 0777 are ignored by
diff --git a/src/src/exim.c b/src/src/exim.c
index 7c9aa0e..2dbc411 100644
--- a/src/src/exim.c
+++ b/src/src/exim.c
@@ -473,8 +473,6 @@ return f;
}
-
-
/*************************************************
* Ensure stdin, stdout, and stderr exist *
*************************************************/
@@ -687,6 +685,36 @@ vfprintf(stderr, fmt, ap);
exit(EXIT_FAILURE);
}
+/* exim_chown_failure() called from exim_chown()/exim_fchown() on failure
+of chown()/fchown(). See src/functions.h for more explanation */
+int
+exim_chown_failure(int fd, const uschar *name, uid_t owner, gid_t group)
+{
+#if 1
+log_write(0, LOG_MAIN|LOG_PANIC,
+ __FILE__ ":%d: chown(%s, %d:%d) failed (%s)."
+ " Please contact the authors and refer to
https://bugs.exim.org/show_bug.cgi?id=2391",
+ __LINE__, name?name:US"<unknown>", owner, group, strerror(errno));
+#else
+/* I leave this here, commented, in case the "bug"(?) comes up again.
+ It is not an Exim bug, but we can provide a workaround.
+ See Bug 2391
+ HS 2019-04-18 */
+
+int saved_errno = errno; /* from the preceeding chown call */
+struct stat buf;
+
+if (0 == (fd < 0 ? stat(name, &buf) : fstat(fd, &buf)))
+{
+ if (buf.st_uid == owner && buf.st_gid == group) return 0;
+ log_write(0, LOG_MAIN|LOG_PANIC, "Wrong ownership on %s", name);
+}
+else log_write(0, LOG_MAIN|LOG_PANIC, "Stat failed on %s: %s", name, strerror(errno));
+
+errno = saved_errno;
+return -1;
+#endif
+}
/*************************************************
diff --git a/src/src/functions.h b/src/src/functions.h
index 4193cac..c5af516 100644
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -10,6 +10,8 @@
to avoid having a lot of tiddly little headers with only a couple of lines in
them. However, some functions that are used (or not used) by utility programs
are in in fact in separate headers. */
+#ifndef _FUNCTIONS_H_
+#define _FUNCTIONS_H_
#ifdef EXIM_PERL
@@ -209,6 +211,10 @@ extern BOOL enq_start(uschar *, unsigned);
extern uschar *event_raise(uschar *, const uschar *, uschar *);
extern void msg_event_raise(const uschar *, const address_item *);
#endif
+
+extern int exim_chown_failure(int, const uschar*, uid_t, gid_t);
+inline int exim_chown(const uschar*, uid_t, gid_t);
+inline int exim_fchown(int, uid_t, gid_t, const uschar*);
extern const uschar * exim_errstr(int);
extern void exim_exit(int, const uschar *);
extern void exim_nullstd(void);
@@ -585,6 +591,39 @@ extern void version_init(void);
extern BOOL write_chunk(transport_ctx *, uschar *, int);
extern ssize_t write_to_fd_buf(int, const uschar *, size_t);
+/* exim_chown - in some NFSv4 setups *seemes* to be an issue with
+ chown(<exim-uid>, <exim-gid>).
+
+ Probably because the idmapping is broken, misconfigured or set up in
+ an unusal way. (see Bug 2931). As I'm not sure, if this was a single
+ case of misconfiguration, or if there are more such broken systems
+ out, I try to impose as least impact as possible and for now just write
+ a panic log entry pointing to the bug report. You're encouraged to
+ contact the developers, if you experience this issue.
+
+ fd the file descriptor (or -1 if not valid)
+ name the file name for error messages or for file operations,
+ if fd is < 0
+ owner the owner
+ group the group
+
+ returns 0 on success, -1 on failure */
+
+inline int
+exim_fchown(int fd, uid_t owner, gid_t group, const uschar *name)
+{
+return (0 == fchown(fd, owner, group))
+ ? 0 : exim_chown_failure(fd, name, owner, group);
+}
+
+inline int
+exim_chown(const uschar *name, uid_t owner, gid_t group)
+{
+return (0 == chown(name, owner, group))
+ ? 0 : exim_chown_failure(-1, name, owner, group);
+}
+
+#endif /* _FUNCTIONS_H_ */
/* vi: aw
*/
diff --git a/src/src/mytypes.h b/src/src/mytypes.h
index ef45595..4234574 100644
--- a/src/src/mytypes.h
+++ b/src/src/mytypes.h
@@ -79,7 +79,6 @@ functions that are called quite often; for other calls to external libraries
#define Uatol(s) atol(CCS(s))
#define Uchdir(s) chdir(CCS(s))
#define Uchmod(s,n) chmod(CCS(s),n)
-#define Uchown(s,n,m) chown(CCS(s),n,m)
#define Ufgets(b,n,f) fgets(CS(b),n,f)
#define Ufopen(s,t) fopen(CCS(s),CCS(t))
#define Ulink(s,t) link(CCS(s),CCS(t))
diff --git a/src/src/receive.c b/src/src/receive.c
index 64f6275..701d540 100644
--- a/src/src/receive.c
+++ b/src/src/receive.c
@@ -3086,7 +3086,7 @@ if ((data_fd = Uopen(spool_name, O_RDWR|O_CREAT|O_EXCL, SPOOL_MODE)) < 0)
/* Make sure the file's group is the Exim gid, and double-check the mode
because the group setting doesn't always get set automatically. */
-if (fchown(data_fd, exim_uid, exim_gid))
+if (0 != exim_fchown(data_fd, exim_uid, exim_gid, spool_name))
log_write(0, LOG_MAIN|LOG_PANIC_DIE,
"Failed setting ownership on spool file %s: %s",
spool_name, strerror(errno));
@@ -4098,7 +4098,7 @@ if (message_logs && !blackholed_by)
{
int fd;
uschar * m_name = spool_fname(US"msglog", message_subdir, message_id, US"");
-
+
if ( (fd = Uopen(m_name, O_WRONLY|O_APPEND|O_CREAT, SPOOL_MODE)) < 0
&& errno == ENOENT
)
@@ -4257,7 +4257,7 @@ if(!smtp_reply)
if (f.deliver_freeze) log_write(0, LOG_MAIN, "frozen by %s", frozen_by);
if (f.queue_only_policy) log_write(L_delay_delivery, LOG_MAIN,
"no immediate delivery: queued%s%s by %s",
- *queue_name ? " in " : "", *queue_name ? CS queue_name : "",
+ *queue_name ? " in " : "", *queue_name ? CS queue_name : "",
queued_by);
}
f.receive_call_bombout = FALSE;
diff --git a/src/src/spool_out.c b/src/src/spool_out.c
index a4a734a..46a490a 100644
--- a/src/src/spool_out.c
+++ b/src/src/spool_out.c
@@ -92,7 +92,7 @@ double-check the mode because the group setting doesn't always get set
automatically. */
if (fd >= 0)
- if (fchown(fd, exim_uid, exim_gid) || fchmod(fd, SPOOL_MODE))
+ if (exim_fchown(fd, exim_uid, exim_gid, temp_name) || fchmod(fd, SPOOL_MODE))
{
DEBUG(D_any) debug_printf("failed setting perms on %s\n", temp_name);
(void) close(fd); fd = -1;
diff --git a/src/src/tls-gnu.c b/src/src/tls-gnu.c
index ad55f95..fe048ba 100644
--- a/src/src/tls-gnu.c
+++ b/src/src/tls-gnu.c
@@ -705,7 +705,7 @@ if (rc < 0)
temp_fn = string_copy(US"%s.XXXXXXX");
if ((fd = mkstemp(CS temp_fn)) < 0) /* modifies temp_fn */
return tls_error_sys(US"Unable to open temp file", errno, NULL, errstr);
- (void)fchown(fd, exim_uid, exim_gid); /* Probably not necessary */
+ (void)exim_chown(temp_fn, exim_uid, exim_gid); /* Probably not necessary */
/* GnuTLS overshoots!
* If we ask for 2236, we might get 2237 or more.
diff --git a/src/src/transports/appendfile.c b/src/src/transports/appendfile.c
index 1e92add..d9f8d49 100644
--- a/src/src/transports/appendfile.c
+++ b/src/src/transports/appendfile.c
@@ -1798,7 +1798,7 @@ if (!isdirectory)
/* We have successfully created and opened the file. Ensure that the group
and the mode are correct. */
- if(Uchown(filename, uid, gid) || Uchmod(filename, mode))
+ if(exim_chown(filename, uid, gid) || Uchmod(filename, mode))
{
addr->basic_errno = errno;
addr->message = string_sprintf("while setting perms on mailbox %s",
@@ -2606,7 +2606,7 @@ else
/* Why are these here? Put in because they are present in the non-maildir
directory case above. */
- if(Uchown(filename, uid, gid) || Uchmod(filename, mode))
+ if(exim_chown(filename, uid, gid) || Uchmod(filename, mode))
{
addr->basic_errno = errno;
addr->message = string_sprintf("while setting perms on maildir %s",
@@ -2652,7 +2652,7 @@ else
/* Why are these here? Put in because they are present in the non-maildir
directory case above. */
- if(Uchown(filename, uid, gid) || Uchmod(filename, mode))
+ if(exim_chown(filename, uid, gid) || Uchmod(filename, mode))
{
addr->basic_errno = errno;
addr->message = string_sprintf("while setting perms on file %s",
@@ -2749,7 +2749,7 @@ else
Uunlink(filename);
return FALSE;
}
- if(Uchown(dataname, uid, gid) || Uchmod(dataname, mode))
+ if(exim_chown(dataname, uid, gid) || Uchmod(dataname, mode))
{
addr->basic_errno = errno;
addr->message = string_sprintf("while setting perms on file %s",
@@ -2764,7 +2764,7 @@ else
/* In all cases of writing to a new file, ensure that the file which is
going to be renamed has the correct ownership and mode. */
- if(Uchown(filename, uid, gid) || Uchmod(filename, mode))
+ if(exim_chown(filename, uid, gid) || Uchmod(filename, mode))
{
addr->basic_errno = errno;
addr->message = string_sprintf("while setting perms on file %s",
