[PATCH] memset_s() and talloc_set_secure()

[Jeremy Allison]

On Fri, Oct 12, 2018 at 02:14:03PM +0200, Andreas Schneider wrote:
> On Thursday, 11 October 2018 21:06:20 CEST Jeremy Allison wrote:
> > On Fri, Oct 12, 2018 at 07:57:31AM +1300, Andrew Bartlett via samba-
> technical wrote:
> > > This still doesn't handle realloc correctly.  The current code there
> > > only handles the non-pool case when ALWAYS_REALLOC is defined, which
> > > never is.
> > > 
> > > You need to force all talloc_realloc() calls on secret memory though
> > > this path, as realloc() won't let you zero the memory after, and unlike
> > > the talloc header (which we overstamp and then restore) you don't do
> > > something similar with the content.
> > > 
> > > talloc_realloc() is rare, and talloc_set_secret() is even more rare.  I
> > > suggest just banning the combination and checking for things like
> > > talloc_realloc() and talloc_asprintf_append() in the small number of
> > > impacted code paths manually.
> > 
> > +1 on this design decision. This is a very narrow talloc use
> > case and it would be better to restrict it to avoid internal
> > complexity.
> > 
> > We've made this mistake before, let's not make it again.
> > All the talloc code around talloc_set_memlimit() is a
> > horrible increase in complexity for little (not at all
> > in Samba) used functionality.
> 
> I've improved the code with metze today. talloc_realloc() is also working with 
> talloc_keep_secret() now. I've also added a test for it.
> 
> See attached patchset.
> 
> Please review, we need more eyes on this.

Andreas, can you give me the use-case for this being in talloc ?

I can see the need for memset_s(), but why should this be inside
talloc rather than being called from specific talloc destructors ?

Jeremy.