[PATCH] minor tdb fixes

Ralph Böhme slow at samba.org
Thu Aug 24 10:32:03 UTC 2017


On Tue, Aug 22, 2017 at 01:28:58PM -0700, Jeremy Allison wrote:
> On Mon, Jul 10, 2017 at 12:00:05PM +0200, Ralph Böhme via samba-technical wrote:
> > On Mon, Jul 03, 2017 at 10:25:19AM +0200, Ralph Böhme wrote:
> > > On Mon, Jul 03, 2017 at 10:06:28AM +0200, Stefan Metzmacher wrote:
> > > > > Attached is a small patchset that fixes the BUCKET() macro in tdb and the offset
> > > > > calculations.
> > > > > 
> > > > > I stumpled upon this when looking at the `tdbtool DB list` output: the freelist
> > > > > was showing the entries from another chain.
> > > > > 
> > > > > Turns out that the BUCKET(-1) macro returns wrong results because of sign
> > > > > conversion, eg -1 is sign converted to an unsigned int 4294967295 and 4294967295
> > > > > % 10 = 5. The expected result is -1, not 5.
> > > > > 
> > > > > This doesn't cause more severe issues because we consistenly lock the wrong
> > > > > chain when trying to lock the freelist. It doesn't deadlock because in tdb_store
> > > > > we lock the freelist before the hashchain. And finally in freelist.c we use the
> > > > > FREELIST_TOP define directly, not the TDB_HASH_TOP macro that uses BUCKET.
> > > > > 
> > > > > Thoughts?
> > > > 
> > > > Sadly it's now a feature as existing tdb versions use this logic.
> > > > So we can't just fix this as it's an incompatible change.
> > > 
> > > oh, that sucks, but you're right.
> > > 
> > > I'll prepare patches that fix `tdbtool DB list` differently and that document
> > > the broken BUCKET macro.
> > 
> > attached.
> > 
> > Please review & push if ok. Thanks!
> 
> Finally got time to go through this. LGTM. Pushed !

oh, I had forgotten about this one. Thanks!! :)

-slow



More information about the samba-technical mailing list