Re: [libvirt] [PATCH 1/4] util: alloc: Remove pointless clearing of variable in AUTOPTR_FUNCs

On Tue, Feb 26, 2019 at 13:30:55 +0000, Daniel Berrange wrote: > On Tue, Feb 26, 2019 at 02:10:02PM +0100, Peter Krempa wrote: > > On Mon, Feb 25, 2019 at 15:18:53 +0100, Erik Skultety wrote: > > > On Fri, Feb 22, 2019 at 05:04:37PM +0100, Peter Krempa wrote: > > > > VIR_DEFINE_AUTOPTR_FUNC defines a function which is supposed to free the > > > > resources for the given VIR_AUTOPTR variable. Given that the cleanup > > > > function is executed when the stack frame is being destroyed it does not > > > > make much sense to set the pointer to NULL. > > > > > > Although correct, the current code is correct too and I really don't see any > > > benefit behind removing a single line where you reset a variable to NULL, if > > > anything, this adds more safety regardless of scenario (defensive programming > > > in general). > > > > Well I'm not against defensive tactics in programming but resetting a > > a pointer value to NULL when the pointer is leaving scope is almost as > > useful as using a hard hat to prevent drowning in a pool. > > > > There is no way to use the variable which would not be a completely > > different class of bug. Also in no way does this keep the stack "tidy". > > All stacked variables must still be considered uninitialized even if > > we'd make sure that everything resets it's stack copies. > > In "normal" execution setting the variable to NULL is indeed useless > but the benefit of defensive programming comes in the abnormal execution > scenarios. The stack region being cleared in the current function is the > same chunk of memory that will be used for stack in the next function > the caller invokes. Having the stack littered with data from variables > that are now out of scope is very useful to people exploiting security > bugs. Zero'ing out pointers and memory is a worthwhile thing todo even I will not buy that this is a security thing as long as VIR_AUTOFREE and VIR_FREE are a thing. It sounds great that we try to zero out the pointers but if we keep the values on the heap it does not buy that much.