From: Heiko Schlittermann (HS12-RIPE) Date: Mon, 4 Feb 2019 21:01:36 +0000 (+0100) Subject: Intercept chown()/fchown() failure and emit a pointer to the bugreport. Closes 2391 X-Git-Url: https://git.exim.org/users/heiko/exim.git/commitdiff_plain/b66fecb428871a3eb274d9370671f1eaf8c5ccec?ds=inline 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. --- diff --git a/src/src/dbfn.c b/src/src/dbfn.c index 336cfe73e..5555c710b 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 c1396a7f7..696effdee 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 e5b655186..2d4d565f4 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 7c9aa0e3f..2dbc41162 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"", 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 4193caccb..c5af51652 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(, ). + + 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 ef455958c..4234574c9 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 64f62757d..701d540b0 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 a4a734a1a..46a490a93 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 ad55f95cd..fe048ba62 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 1e92add35..d9f8d4989 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",