Re: geli(8) breaks after a couple hours of uptime

On Sun, 2013-02-10 at 00:35 +0100, Pawel Jakub Dawidek wrote:
> On Sat, Feb 09, 2013 at 06:00:53PM +0200, Andriy Gapon wrote:
> > on 09/02/2013 17:04 Andrey Zonov said the following:
> > > On 2/9/13 5:07 PM, Fabian Keil wrote:
> > >>
> > >> This would at least prevent the segfault.
> > >>
> > > 
> > > I see two possibilities to get segfault:
> > >   - no checking for result from memory allocation functions
> > >   - too big stack
> > > 
> > > I have no found any broken memory allocation checking, but I found two
> > > big objects on the stack.  One is buf[MAXPHYS] in eli_genkey_files() and
> > > another is passbuf[MAXPHYS] in eli_genkey_passphrase().  If we change
> > > these two to malloc(), then we can handle error from malloc(), print
> > > some useful message and prevent segfault.
> > 
> > I'd rather do what Kostik suggested and Fabian mentioned: instead of
> > mlockall(MCL_FUTURE) the code should mlock only the (explicitly designated)
> > buffers that can contain sensitive data.
> 
> geli(8) almost exclusively deals with sensitive data. Even mlocking
> MAXPHYS would fail with current limits, but this is bad idea.
> 
> With mlockall() I am sure I didn't miss anything - be it forgetting
> about mlocking some buffer or zeroing it before munlock. I'm also sure
> someone else who can modify geli(8) in the future won't miss anything
> too.
> 
> geli(8) is relatively simple program, it doesn't allocate megabytes of
> memory for different pruposes and just needs few pages for sensitive
> data. As I said most of the memory it uses is for sensitive data.
> 
> The obvious problem is allocating MAXPHYS on the stack. This has to be
> changed, especially that we may want to rise MAXPHYS in the future.
> 
> Other than that I expect thing would be tuned properly so that geli(8)
> can work by default. I'm happy to use smaller buffers than MAXPHYS -
> keyfiles are far smaller usually than 128kB, so there shouldn't be any
> issue with this.
> 

If geli(8) uses relatively little memory and mlockall(MCL_FUTURE), you
should consider adding a jemalloc tuning string to it, similar to what I
did for watchdogd in r245949.  It doesn't help with locking code when
you only really need the data protected, but it does at least reduce the
amount of wired memory to just what the program really needs.

-- Ian