From owner-freebsd-current@FreeBSD.ORG Mon Feb 11 15:10:42 2013 Return-Path: Delivered-To: current@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 9BF8188C; Mon, 11 Feb 2013 15:10:42 +0000 (UTC) (envelope-from ian@FreeBSD.org) Received: from duck.symmetricom.us (duck.symmetricom.us [206.168.13.214]) by mx1.freebsd.org (Postfix) with ESMTP id 2C26D816; Mon, 11 Feb 2013 15:10:41 +0000 (UTC) Received: from damnhippie.dyndns.org (daffy.symmetricom.us [206.168.13.218]) by duck.symmetricom.us (8.14.6/8.14.6) with ESMTP id r1BFAfsI065504; Mon, 11 Feb 2013 08:10:41 -0700 (MST) (envelope-from ian@FreeBSD.org) Received: from [172.22.42.240] (revolution.hippie.lan [172.22.42.240]) by damnhippie.dyndns.org (8.14.3/8.14.3) with ESMTP id r1BFAdI0039627; Mon, 11 Feb 2013 08:10:39 -0700 (MST) (envelope-from ian@FreeBSD.org) Subject: Re: geli(8) breaks after a couple hours of uptime From: Ian Lepore To: Pawel Jakub Dawidek In-Reply-To: <20130209233500.GH1375@garage.freebsd.pl> References: <20130207141833.GA15884@acme.spoerlein.net> <20130207153322.5c371beb@fabiankeil.de> <20130207180153.GX35868@acme.spoerlein.net> <20130208095709.6ae61cff@fabiankeil.de> <20130208114825.GY35868@acme.spoerlein.net> <5114F390.4010302@FreeBSD.org> <20130209140733.0b753c60@fabiankeil.de> <51166580.4080603@FreeBSD.org> <511672B5.5080300@FreeBSD.org> <20130209233500.GH1375@garage.freebsd.pl> Content-Type: text/plain; charset="us-ascii" Date: Mon, 11 Feb 2013 08:10:39 -0700 Message-ID: <1360595439.4545.84.camel@revolution.hippie.lan> Mime-Version: 1.0 X-Mailer: Evolution 2.32.1 FreeBSD GNOME Team Port Content-Transfer-Encoding: 7bit Cc: Eitan Adler , Andrey Zonov , current@FreeBSD.org, Andriy Gapon X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 11 Feb 2013 15:10:42 -0000 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