[PATCH 3/5] dix: set pointer to NULL after freeing at CloseDevice

[Mark Kettenis]

> Date: Tue, 05 Apr 2011 14:27:02 +0300
> From: Tiago Vignatti <tiago.vignatti at nokia.com>
> 
> On 04/05/2011 03:14 PM, ext Simon Thum wrote:
> > On 04/04/2011 07:54 PM, Tiago Vignatti wrote:
> >> It will fix two possible cases of use after free in RemoveDevice.
> >>
> >> Signed-off-by: Tiago Vignatti<tiago.vignatti at nokia.com>
> >> ---
> >>   dix/devices.c |    1 +
> >>   1 files changed, 1 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/dix/devices.c b/dix/devices.c
> >> index 534931c..0288e15 100644
> >> --- a/dix/devices.c
> >> +++ b/dix/devices.c
> >> @@ -941,6 +941,7 @@ CloseDevice(DeviceIntPtr dev)
> >>       free(dev->config_info);     /* Allocated in xf86ActivateDevice. */
> >>       dev->config_info = NULL;
> >>       dixFreeObjectWithPrivates(dev, PRIVATE_DEVICE);
> >> +    dev = NULL;
> >>   }
> >>
> >>   /**
> > OK, but _how_ does it do what you say it does? I'm just seeing a dead
> > store to a local.
> 
> if you check RemoveDevice, you will see two loops (for) with a 
> conditional (tmp = dev). If one of these conditional takes true path it 
> will call CloseDevice(tmp), which in turn will free(tmp) through 
> dixFreeObjectWithPrivates(dev, PRIVATE_DEVICE). Next, when it returns, 
> the conditional to stop the loop in RemoveDevice will test the value of 
> tmp, which cannot be done because was previously freed.
> 
> hope you understood my explanation...

Nope.  Your explanation can't be right.  A dead store is a dead store.
Any decent compiler will optimize away the statement you add, since it
modifies a function argument (which are local variables in C since
they're passed by value) and that argument isn't used in this function
after you assign NULL to it.