reconstruct caused mailboxes (skiplist) corruption?

[Henrique de Moraes Holschuh]

On Fri, 12 Nov 2010, Bron Gondwana wrote:
> On Thu, Nov 11, 2010 at 11:58:04PM -0200, Henrique de Moraes Holschuh wrote:
> > It _will_ write to stderr (aka fd 2).  If we want to be safe, we make sure
> > fds 0-2 are sane, and we check when we open sockets/files that we did not
> > get fds below 3...
> > 
> > > Bron ( a while later, fd 2 gets re-used as the mailboxes.db handle, and hence
> > >        the mess is created )
> > 
> > Indeed.
> > 
> > We *CANNOT* afford to have any files or sockets opened with fd 0, 1 or 2. We
> > should core-dump immediately if that happens, I think.
> 
> How about this skanky patch (attached?) - checks fds at the start, and if it
> gets fd 2, it holds it open (to /dev/null) for the life of the process, making
> sure nothing else gets it.  If it gets 0 or 1 it just croaks.

IMHO we should be even more paranoid.  Create a xfopen() that we use
anywhere where we are not explicitly dealing with fd 0, 1 or 2.  If xfopen()
detects the problem, it should dump info to syslog priority error and
request that the report is sent to us.  And, of course, return -1 (or, if
you prefer, heal the fds).

That should help us find out why sometimes one of those fds just disappear.
I don't think it is just a case of truss doing something idiotic, we have
had spurious (and _rare_) problem reports where it clearly happened over the
years...

Maybe the truss issue is FreeBSD breakage, but the weirdness has happened on
Linux as well in the past.

> From 5a6433511db0002227aad069ee9e92c34932879a Mon Sep 17 00:00:00 2001
> From: Bron Gondwana <brong at opera.com>
> Date: Fri, 12 Nov 2010 14:05:02 +1100
> Subject: [PATCH] Protect STDERR on FreeBSD
> 
> ---
>  lib/libcyr_cfg.c |   20 ++++++++++++++++++++
>  1 files changed, 20 insertions(+), 0 deletions(-)
> 
> diff --git a/lib/libcyr_cfg.c b/lib/libcyr_cfg.c
> index 83a376c..d8c6986 100644
> --- a/lib/libcyr_cfg.c
> +++ b/lib/libcyr_cfg.c
> @@ -59,6 +59,8 @@
>  #define CFGVAL(t,v)	{(void *)(v)}
>  #endif
>  
> +static int protect_stderr = -1;
> +
>  struct cyrusopt_s cyrus_options[] = {
>      { CYRUSOPT_ZERO, { NULL }, CYRUS_OPT_NOTOPT },
>  
> @@ -221,10 +223,28 @@ void libcyrus_config_setswitch(enum cyrus_opt opt, int val)
>  
>  void libcyrus_init()
>  {
> +    protect_stderr = open("/dev/null", O_RDWR, 0666);
> +    if (protect_stderr > 2) {
> +	/* Ok, we're safe */
> +	close(protect_stderr);
> +	protect_stderr = -1;
> +    }
> +    else if (protect_stderr == 2) {
> +	syslog(LOG_ERR, "WARNING: Protecting stderr from dangerous re-open. "
> +			"Are you running under broken truss on FreeBSD?");
> +    }
> +    else {
> +	abort();
> +    }
> +
>      cyrusdb_init();
>  }
>  
>  void libcyrus_done()
>  {
>      cyrusdb_done();
> +    if (protect_stderr > -1) {
> +	close(protect_stderr);
> +	protect_stderr = -1;
> +    }
>  }


-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh