[PATCHES BUG 13865] memcache tracking of talloc object sizes

Jeremy Allison jra at samba.org
Fri Mar 29 21:10:37 UTC 2019


On Fri, Mar 29, 2019 at 01:57:45PM -0700, Jeremy Allison via samba-technical wrote:
> On Fri, Mar 29, 2019 at 01:07:42PM -0700, Jeremy Allison wrote:
> > On Fri, Mar 29, 2019 at 01:00:33PM -0700, Jeremy Allison via samba-technical wrote:
> > 
> > > Actually I'm wrong - we can't mix talloc/non-talloc
> > > stores here (it crashes, I tested :-). But we still
> > > need to take the talloc'ed size into account.
> > > 
> > > > > 
> > > > > 281                                 TALLOC_FREE(ptr);
> > > > > 282                         }
> > > > > 283                         /*
> > > > > 284                          * We can reuse the existing record
> > > > > 285                          */
> > > > > 286                         memcpy(cache_value.data, value.data, value.length);
> > > > > 287                         e->valuelength = value.length;
> > > > > 288                         return;
> > > > > 289                 }
> > > > > 
> > > > > Probably need to add a test for this too...
> > > > 
> > > > Yes, will do. I will be busy with something else this afternoon, but i
> > > > should have an update for this early next week.
> > > 
> > > I'm nearly done with the fix and test. If I can
> > > get it working I'll post for your review.
> > 
> > Here is the updated patchset containing the fix for the
> > corner case and additional test for it. I checked that
> > with the corner case fix commented out the test fails,
> > but passes with it added.
> > 
> > CI build here:
> > 
> > https://gitlab.com/samba-team/devel/samba/pipelines/54311307
> > 
> > Please review and push if happy !
> 
> Actually this patchset makes:
> 
> make test TESTS=samba3.base.rw1
> 
> fail repeatedly. Interesting, I'm trying to track
> down why (wouldn't have thought it had an effect,
> but it clearly does :-).

Oh this is interesting. Error is in:

==189102== Invalid read of size 8
==189102==    at 0x715DF8E: _tc_free_internal (talloc.c:1162)
==189102==    by 0x715F341: _tc_free_children_internal (talloc.c:1666)
==189102==    by 0x715E1B2: _tc_free_internal (talloc.c:1183)
==189102==    by 0x715F341: _tc_free_children_internal (talloc.c:1666)
==189102==    by 0x715E1B2: _tc_free_internal (talloc.c:1183)
==189102==    by 0x715E428: _talloc_free_internal (talloc.c:1247)
==189102==    by 0x715F78E: _talloc_free (talloc.c:1789)
==189102==    by 0x5A5CAC7: open_file_ntcreate (open.c:3851)
==189102==    by 0x5A6008F: create_file_unixpath (open.c:5298)
==189102==    by 0x5A60D55: create_file_default (open.c:5706)
==189102==    by 0x5BDE1E5: vfswrap_create_file (vfs_default.c:582)
==189102==    by 0x5A6D54B: smb_vfs_call_create_file (vfs.c:1627)
==189102==  Address 0x1f9cd460 is 48 bytes inside a block of size 248 free'd
==189102==    at 0x4C2CDBB: free (vg_replace_malloc.c:530)
==189102==    by 0x715E370: _tc_free_internal (talloc.c:1221)
==189102==    by 0x715E428: _talloc_free_internal (talloc.c:1247)
==189102==    by 0x715F78E: _talloc_free (talloc.c:1789)
==189102==    by 0x569D681: memcache_delete_element (memcache.c:213)
==189102==    by 0x569D73E: memcache_trim (memcache.c:228)
==189102==    by 0x569DC54: memcache_do_add (memcache.c:334)
==189102==    by 0x569DD7D: memcache_add_talloc (memcache.c:357)
==189102==    by 0x5B2D1ED: share_mode_memcache_store (share_mode_lock.c:185)
==189102==    by 0x5B2DC72: share_mode_data_destructor (share_mode_lock.c:442)
==189102==    by 0x715DF81: _tc_free_internal (talloc.c:1157)
==189102==    by 0x715F341: _tc_free_children_internal (talloc.c:1666)
==189102==  Block was alloc'd at
==189102==    at 0x4C2BB8F: malloc (vg_replace_malloc.c:299)
==189102==    by 0x715D324: __talloc_with_prefix (talloc.c:782)
==189102==    by 0x715D4BE: __talloc (talloc.c:824)
==189102==    by 0x715D944: _talloc_named_const (talloc.c:981)
==189102==    by 0x7160D9D: _talloc_zero (talloc.c:2422)
==189102==    by 0x5B2DCDC: fresh_share_mode_lock (share_mode_lock.c:463)
==189102==    by 0x5B2DF31: get_share_mode_lock_internal (share_mode_lock.c:519)
==189102==    by 0x5B2E1EE: get_share_mode_lock (share_mode_lock.c:578)
==189102==    by 0x5A5BC7E: open_file_ntcreate (open.c:3448)
==189102==    by 0x5A6008F: create_file_unixpath (open.c:5298)
==189102==    by 0x5A60D55: create_file_default (open.c:5706)
==189102==    by 0x5BDE1E5: vfswrap_create_file (vfs_default.c:582)

which means that we have an outstanding bug w.r.t.
storing share modes in the memcache - if they get
evicted bad things happen.

Changing the memcache to keep track of talloced sizes
without changing the total memcache size is bad, but
this has revealed an underlying bug w.r.t thinking
the memcache doesn't free entries.

Looking closer..



More information about the samba-technical mailing list