[PATCH] memset_s() and talloc_set_secure()

Jeremy Allison jra at samba.org
Thu Oct 11 19:06:20 UTC 2018


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.

Just my 2cents.

Jeremy



More information about the samba-technical mailing list