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

on 10/02/2013 17:44 Pawel Jakub Dawidek said the following:
> On Sun, Feb 10, 2013 at 09:50:58AM +0200, Andriy Gapon wrote:
>> on 10/02/2013 01:35 Pawel Jakub Dawidek said the following:
>>> 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.
>>
>> Well, the geli is not such a complex program really.  It seems to use only two
>> or so buffers for sensitive data. [...]
> 
> Maybe it isn't very complex, but complex enough that you missed a dozen
> or so buffers that would need mlocking (almost everything that is
> bzero'ed),

I haven't exactly missed them, because I only glanced over the code.

> not to mention internal states for hash and encryption
> algorithms that operate on blocks, so they can keep plain data until
> their update method gather entire block. Encryption and HMAC calculation
> is done by API used by both userland and kernel parts, so it would need
> some ifdefs to make it work, thus further complicating entire thing.

I think that things such as these are better be done in externally
provided/controlled buffers.

>> [...] As far as I can see geli deals only with some
>> key management (reading keys, generating key from key material, etc). There is
>> definitely no need to mlock the code, etc.
> 
> I fully agree there is no need to mlock the code and I'd be happy to use
> mlockall(2) flag that protects only the data. Until such flag is
> introduced I'll keep mlocking everything.
> 
>> I think that PAGE_SIZE (or at most a small multiple of it) should be sufficient.
>> I don't think that we currently have (or expect to see in the near future)
>> algorithms where keys with more than 4096 size provide any additional security.
> 
> geli(8) deals just fine with files that are larger than buffers, so even
> with smaller buffer it can read the data in few steps.
> 
> The proposed patch is here if someone would like to give it a try:
> 
> 	http://people.freebsd.org/~pjd/patches/geom_eli.c.patch
> 

This is a very good start, IMHO.
Thank you.

-- 
Andriy Gapon