Crash in CLEAR_IF_FIRST handling in tdb

[Rusty Russell]

Volker Lendecke <Volker.Lendecke at SerNet.DE> writes:
> On Wed, Oct 03, 2012 at 05:18:53PM +0930, Rusty Russell wrote:
>> Volker Lendecke <Volker.Lendecke at SerNet.DE> writes:
>> 
>> > Hi Rusty!
>> >
>> > Under
>> > http://git.samba.org/?p=vl/samba.git/.git;a=shortlog;h=refs/heads/tdb
>> > find a patchset that for me fixes a crash in winbind in tdb.
>> > For the explanation, see the second patch from the top.
>> 
>> Good catch!
>> 
>> Your fix here should stop a crash when accessing the header, but the
>> rest of the mmaped database is still going to cause SEGV, no?  We only
>> re-map it when we see an offset which is out-of-bounds (vs the
>> locally-cache tdb->map_size variable).
>
> I believe we are okay with the fix. Whenever we access the
> mmap area pointed to by the hash pointed to by the hash
> sources, we lock the hash chain. I think we never keep
> information beyond a F_UNLK. Because tdb_new_database
> establishes an empty and correct database, tdb_fetch will
> never randomly access memory beyond the current file limit.
> The freelist is also set up with a short database in mind.
> So whenever there is a need to go beyond the file size,
> which might be shorter than the current mmap size, we will
> end up in tdb_expand, which now should deal fine with a
> shrunk file.
>
> Please tell me if I missed something!

No, you're absolutely right!

It's possible that the new database has a different hash, or that db
corruption will make us reference a now-invalid offset, but your
solution solves 99.999% without any real penalty.

Great work!

Thanks,
Rusty.